catalog: modulo AcoustID lookup + move to <year>/<genre>/

tests/test_acoustid.py

@@ -0,0 +1,204 @@
+"""Test per core.acoustid — lookup AcoustID + cache SQLite.
+
+Mock su `requests.get` in modo che i test siano offline. Ogni test
+isola la cache SQLite in tmp_path via monkeypatch di
+`_cache_db_path`.
+"""
+
+from __future__ import annotations
+
+from unittest import mock
+
+import pytest
+
+from core import acoustid
+
+
+# ------------------------------------------------------------------
+# Fixture: cache isolata + reset del rate-limit
+# ------------------------------------------------------------------
+@pytest.fixture
+def patched_cache(tmp_path, monkeypatch):
+    db = tmp_path / "acoustid_cache_test.db"
+    monkeypatch.setattr(acoustid, "_cache_db_path", lambda: db)
+    # Rate-limit: azzera cosi' i test non aspettano throttle
+    monkeypatch.setattr(acoustid, "_RATE_LIMIT_SEC", 0.0)
+    monkeypatch.setattr(acoustid, "_last_request_at", [0.0])
+    return db
+
+
+def _mock_resp(status_code: int = 200, json_data: dict = None):
+    """Costruisce un mock di response `requests`."""
+    m = mock.Mock()
+    m.status_code = status_code
+    m.json.return_value = json_data or {}
+    return m
+
+
+# ------------------------------------------------------------------
+# lookup — cache
+# ------------------------------------------------------------------
+class TestLookupCache:
+    def test_lookup_uses_cache(self, patched_cache):
+        """Seconda chiamata con lo stesso fingerprint riusa la cache."""
+        payload = {
+            "status": "ok",
+            "results": [{
+                "score": 0.99,
+                "recordings": [{
+                    "title": "Some Song",
+                    "artists": [{"name": "Artist X"}],
+                    "releases": [
+                        {"date": {"year": 2005},
+                         "releasegroup": {"tags": [{"name": "House", "count": 5}]}},
+                    ],
+                }],
+            }],
+        }
+        with mock.patch.object(acoustid.requests, "get",
+                                return_value=_mock_resp(200, payload)) as m:
+            r1 = acoustid.lookup("FP-1", 180.0)
+            r2 = acoustid.lookup("FP-1", 180.0)
+
+        assert m.call_count == 1, "la seconda chiamata deve venire dalla cache"
+        assert r1 == r2
+        assert r1["matched"] is True
+        assert r1["year"] == 2005
+        assert r1["genre"].lower() == "house"
+        assert r1["title"] == "Some Song"
+        assert r1["artist"] == "Artist X"
+
+
+# ------------------------------------------------------------------
+# lookup — no results / errori
+# ------------------------------------------------------------------
+class TestLookupNoResults:
+    def test_lookup_no_results_returns_unmatched(self, patched_cache):
+        payload = {"status": "ok", "results": []}
+        with mock.patch.object(acoustid.requests, "get",
+                                return_value=_mock_resp(200, payload)):
+            r = acoustid.lookup("FP-NORESULT", 100.0)
+
+        assert r["matched"] is False
+        # deve essere cacheato
+        with mock.patch.object(acoustid.requests, "get") as m:
+            r2 = acoustid.lookup("FP-NORESULT", 100.0)
+            assert m.call_count == 0
+        assert r2 == r
+
+    def test_lookup_no_recordings_returns_unmatched(self, patched_cache):
+        """results presenti ma senza recordings -> matched=False."""
+        payload = {
+            "status": "ok",
+            "results": [{"score": 0.5, "recordings": []}],
+        }
+        with mock.patch.object(acoustid.requests, "get",
+                                return_value=_mock_resp(200, payload)):
+            r = acoustid.lookup("FP-EMPTYREC", 200)
+        assert r["matched"] is False
+
+
+# ------------------------------------------------------------------
+# lookup — anno minimo
+# ------------------------------------------------------------------
+class TestLookupYearExtraction:
+    def test_lookup_extracts_min_year(self, patched_cache):
+        """3 releases (2003, 1998, 2010) -> year=1998."""
+        payload = {
+            "status": "ok",
+            "results": [{
+                "score": 0.95,
+                "recordings": [{
+                    "title": "Classic",
+                    "artists": [{"name": "Artist"}],
+                    "releases": [
+                        {"date": {"year": 2003}},
+                        {"date": {"year": 1998}},
+                        {"date": {"year": 2010}},
+                    ],
+                }],
+            }],
+        }
+        with mock.patch.object(acoustid.requests, "get",
+                                return_value=_mock_resp(200, payload)):
+            r = acoustid.lookup("FP-YEAR", 180)
+
+        assert r["matched"] is True
+        assert r["year"] == 1998
+
+    def test_lookup_missing_year_becomes_none(self, patched_cache):
+        """Nessuna release con year valido -> year=None (matched se ha titolo)."""
+        payload = {
+            "status": "ok",
+            "results": [{
+                "score": 0.9,
+                "recordings": [{
+                    "title": "T",
+                    "artists": [{"name": "A"}],
+                    "releases": [{"date": {}}, {"other": 1}],
+                }],
+            }],
+        }
+        with mock.patch.object(acoustid.requests, "get",
+                                return_value=_mock_resp(200, payload)):
+            r = acoustid.lookup("FP-NOY", 100)
+        # matched puo' essere True se ha almeno un titolo
+        assert r["year"] is None
+        assert r["title"] == "T"
+
+
+# ------------------------------------------------------------------
+# lookup — errori HTTP / rete
+# ------------------------------------------------------------------
+class TestLookupErrors:
+    def test_lookup_returns_error_on_http_failure(self, patched_cache):
+        with mock.patch.object(acoustid.requests, "get",
+                                return_value=_mock_resp(500, {})):
+            r = acoustid.lookup("FP-HTTP", 100)
+        assert r["matched"] is False
+        assert "HTTP 500" in r.get("error", "")
+
+    def test_lookup_returns_error_on_network_failure(self, patched_cache):
+        with mock.patch.object(acoustid.requests, "get",
+                                side_effect=acoustid.requests.ConnectionError("boom")):
+            r = acoustid.lookup("FP-NET", 100)
+        assert r["matched"] is False
+        assert "network" in r.get("error", "").lower() or "boom" in r.get("error", "")
+
+    def test_lookup_empty_fingerprint_returns_unmatched(self, patched_cache):
+        r = acoustid.lookup("", 100)
+        assert r["matched"] is False
+        assert "fingerprint" in r.get("error", "").lower()
+
+    def test_lookup_api_status_error(self, patched_cache):
+        payload = {"status": "error",
+                    "error": {"message": "invalid fingerprint"}}
+        with mock.patch.object(acoustid.requests, "get",
+                                return_value=_mock_resp(200, payload)):
+            r = acoustid.lookup("FP-BAD", 100)
+        assert r["matched"] is False
+        assert "invalid" in r.get("error", "").lower()
+
+
+# ------------------------------------------------------------------
+# helper: _extract_top_genre
+# ------------------------------------------------------------------
+class TestGenreExtraction:
+    def test_top_genre_picks_highest_count(self):
+        releases = [{
+            "releasegroup": {
+                "tags": [
+                    {"name": "electronic", "count": 3},
+                    {"name": "house", "count": 12},
+                    {"name": "dance", "count": 7},
+                ]
+            }
+        }]
+        assert acoustid._extract_top_genre(releases).lower() == "house"
+
+    def test_top_genre_no_tags_returns_empty(self):
+        releases = [{"releasegroup": {"tags": []}}]
+        assert acoustid._extract_top_genre(releases) == ""
+
+    def test_top_genre_no_releasegroup_returns_empty(self):
+        assert acoustid._extract_top_genre([{}]) == ""

tests/test_catalog.py

@@ -0,0 +1,205 @@
+"""Test per core.catalog — sanitize + move_files + scan_folder.
+
+Focus principale sui casi in cui il piano ha promesso comportamento
+esplicito: sanitize dei chars vietati, struttura year/genre, gestione
+di anno/genere mancanti, conflitto di filename.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+from unittest import mock
+
+import pytest
+
+from core import catalog
+
+
+# ------------------------------------------------------------------
+# Sanitize
+# ------------------------------------------------------------------
+class TestSanitizeFolder:
+    def test_sanitize_folder_removes_forbidden_chars(self):
+        # `/` (dai tag musicbrainz), `:` (Windows), `?`, `*`, `|`, ecc.
+        s = catalog._sanitize_folder("electronic/house")
+        assert "/" not in s
+        assert "electronic" in s and "house" in s
+
+        s2 = catalog._sanitize_folder("prog:rock?")
+        for ch in '<>:"|?*\\/':
+            assert ch not in s2
+
+    def test_sanitize_folder_collapses_spaces(self):
+        assert catalog._sanitize_folder("  tech  house  ") == "tech house"
+
+    def test_sanitize_folder_empty_returns_empty(self):
+        assert catalog._sanitize_folder("") == ""
+        assert catalog._sanitize_folder(None) == ""
+
+    def test_sanitize_folder_truncates_long(self):
+        s = catalog._sanitize_folder("a" * 300)
+        assert len(s) <= 100
+
+
+# ------------------------------------------------------------------
+# move_files
+# ------------------------------------------------------------------
+def _touch(path: Path, size: int = 8) -> Path:
+    path.parent.mkdir(parents=True, exist_ok=True)
+    path.write_bytes(b"\x00" * size)
+    return path
+
+
+class TestMoveFiles:
+    def test_move_files_creates_year_genre_structure(self, tmp_path):
+        """Un file matched → finisce in <target>/<year>/<genre>/<filename>."""
+        src = _touch(tmp_path / "source" / "song.mp3")
+        target = tmp_path / "cat"
+        entries = [{
+            "path": str(src), "size": src.stat().st_size,
+            "matched": True, "year": 2005, "genre": "House",
+            "artist": "X", "title": "T", "fingerprint": "FP",
+        }]
+
+        res = catalog.move_files(entries, str(target))
+
+        assert res["moved"] == 1
+        assert res["failed"] == []
+        dst = target / "2005" / "House" / "song.mp3"
+        assert dst.exists()
+        assert not src.exists()
+        # Log operations popolato
+        assert res["operations"] and res["operations"][0]["src"] == str(src)
+        assert res["operations"][0]["dst"] == str(dst)
+
+    def test_move_files_handles_missing_year_or_genre(self, tmp_path):
+        """Year/genre mancanti → Unknown Year / Unknown Genre."""
+        no_year = _touch(tmp_path / "src" / "no_year.mp3")
+        no_genre = _touch(tmp_path / "src" / "no_genre.mp3")
+        no_both = _touch(tmp_path / "src" / "no_both.mp3")
+
+        target = tmp_path / "cat"
+        entries = [
+            {"path": str(no_year), "matched": True, "year": None,
+             "genre": "House"},
+            {"path": str(no_genre), "matched": True, "year": 2010,
+             "genre": ""},
+            {"path": str(no_both), "matched": False, "year": None,
+             "genre": ""},
+        ]
+
+        res = catalog.move_files(entries, str(target))
+        assert res["moved"] == 3
+        assert (target / "Unknown Year" / "House" / "no_year.mp3").exists()
+        assert (target / "2010" / "Unknown Genre" / "no_genre.mp3").exists()
+        assert (target / "Unknown Year" / "Unknown Genre"
+                / "no_both.mp3").exists()
+
+    def test_move_files_dedup_conflicting_filenames(self, tmp_path):
+        """Se un file con lo stesso nome esiste gia' nel target, aggiungi
+        suffisso _1, _2, ... (mai overwrite)."""
+        src1 = _touch(tmp_path / "srcA" / "song.mp3", size=10)
+        src2 = _touch(tmp_path / "srcB" / "song.mp3", size=20)
+        target = tmp_path / "cat"
+
+        entries = [
+            {"path": str(src1), "matched": True, "year": 2000,
+             "genre": "Rock"},
+            {"path": str(src2), "matched": True, "year": 2000,
+             "genre": "Rock"},
+        ]
+        res = catalog.move_files(entries, str(target))
+        assert res["moved"] == 2
+        d1 = target / "2000" / "Rock" / "song.mp3"
+        d2 = target / "2000" / "Rock" / "song_1.mp3"
+        assert d1.exists() and d2.exists()
+        # Contenuto preservato dal move (src1 = 10 byte, src2 = 20 byte).
+        # L'ordine di iterazione garantisce che song.mp3 = src1.
+        assert d1.stat().st_size == 10
+        assert d2.stat().st_size == 20
+
+    def test_move_files_sanitizes_forbidden_genre(self, tmp_path):
+        """Genere con `/` (tipico di musicbrainz) va sanitizzato in `_`."""
+        src = _touch(tmp_path / "src" / "song.mp3")
+        target = tmp_path / "cat"
+        entries = [{
+            "path": str(src), "matched": True, "year": 2020,
+            "genre": "electronic/house",
+        }]
+
+        res = catalog.move_files(entries, str(target))
+        assert res["moved"] == 1
+        # NON deve creare "electronic" e dentro "house": e' un solo nome.
+        assert not (target / "2020" / "electronic").is_dir()
+        # La cartella deve contenere entrambe le parti sanitizzate
+        year_dir = target / "2020"
+        subdirs = [p.name for p in year_dir.iterdir() if p.is_dir()]
+        assert len(subdirs) == 1
+        assert "/" not in subdirs[0]
+        assert "electronic" in subdirs[0] and "house" in subdirs[0]
+
+    def test_move_files_missing_source_reports_failed(self, tmp_path):
+        """File che non esiste piu' → finisce in failed, non alza."""
+        target = tmp_path / "cat"
+        entries = [{
+            "path": str(tmp_path / "does_not_exist.mp3"),
+            "matched": True, "year": 2000, "genre": "X",
+        }]
+        res = catalog.move_files(entries, str(target))
+        assert res["moved"] == 0
+        assert len(res["failed"]) == 1
+        assert "non esiste" in res["failed"][0]["error"].lower()
+
+    def test_move_files_empty_list_returns_zero(self, tmp_path):
+        res = catalog.move_files([], str(tmp_path / "cat"))
+        assert res["moved"] == 0
+        assert res["failed"] == []
+        assert res["operations"] == []
+
+
+# ------------------------------------------------------------------
+# scan_folder — smoke integration test (mock fpcalc + lookup)
+# ------------------------------------------------------------------
+class TestScanFolder:
+    def test_scan_folder_empty_directory(self, tmp_path):
+        # Cartella senza file audio → lista vuota, no crash
+        with mock.patch.object(catalog, "find_fpcalc",
+                                return_value="/fake/fpcalc"):
+            entries = catalog.scan_folder(str(tmp_path), recursive=False)
+        assert entries == []
+
+    def test_scan_folder_processes_files(self, tmp_path):
+        # Un file audio fake + mock di fpcalc + lookup
+        (tmp_path / "a.mp3").write_bytes(b"\x00" * 100)
+        (tmp_path / "readme.txt").write_text("hi")
+
+        with mock.patch.object(catalog, "find_fpcalc",
+                                return_value="/fake/fpcalc"), \
+             mock.patch.object(catalog, "compute_fingerprint",
+                                return_value={"fingerprint": "FP", "duration": 100}), \
+             mock.patch.object(catalog, "lookup",
+                                return_value={
+                                    "matched": True, "year": 2018,
+                                    "genre": "House", "artist": "A",
+                                    "title": "T",
+                                }):
+            entries = catalog.scan_folder(str(tmp_path), recursive=False)
+
+        assert len(entries) == 1
+        e = entries[0]
+        assert e["matched"] is True
+        assert e["year"] == 2018
+        assert e["genre"] == "House"
+        assert e["fingerprint"] == "FP"
+
+    def test_scan_folder_records_fpcalc_error(self, tmp_path):
+        """fpcalc fallito → entry con matched=False + error."""
+        (tmp_path / "broken.mp3").write_bytes(b"\x00" * 10)
+        with mock.patch.object(catalog, "find_fpcalc",
+                                return_value="/fake/fpcalc"), \
+             mock.patch.object(catalog, "compute_fingerprint",
+                                return_value={"_error": "fingerprint vuoto"}):
+            entries = catalog.scan_folder(str(tmp_path), recursive=False)
+        assert len(entries) == 1
+        assert entries[0]["matched"] is False
+        assert "vuoto" in entries[0]["error"]