diff --git a/core/imports/paths.py b/core/imports/paths.py index 5ac357d1..9fd9dea7 100644 --- a/core/imports/paths.py +++ b/core/imports/paths.py @@ -191,6 +191,15 @@ def _replace_template_variables(template: str, context: dict) -> str: except Exception: pass + # $cdnum — smart CD label for multi-disc filenames. Produces "CD01" / + # "CD02" etc. when the album has 2+ discs, empty string otherwise. + # Empty output collapses gracefully via the trailing dash cleanup + # regex below, so single-disc albums don't end up with "CD01" literal + # in every name. + _total_discs = _coerce_int(clean_context.get("total_discs", 1), 1) + _disc_number = _coerce_int(clean_context.get("disc_number", 1), 1) + cdnum_value = f"CD{_disc_number:02d}" if _total_discs > 1 else "" + bracket_map = { "albumartist": album_artist_value, "albumtype": clean_context.get("albumtype", "Album"), @@ -200,6 +209,7 @@ def _replace_template_variables(template: str, context: dict) -> str: "album": clean_context.get("album", "Unknown Album"), "title": clean_context.get("title", "Unknown Track"), "track": f"{_coerce_int(clean_context.get('track_number', 1), 1):02d}", + "cdnum": cdnum_value, "disc": str(_coerce_int(clean_context.get("disc_number", 1), 1)), "discnum": str(_coerce_int(clean_context.get("disc_number", 1), 1)), "year": str(clean_context.get("year", "")), @@ -215,6 +225,10 @@ def _replace_template_variables(template: str, context: dict) -> str: result = result.replace("$artist", clean_context.get("artist", "Unknown Artist")) result = result.replace("$album", clean_context.get("album", "Unknown Album")) result = result.replace("$title", clean_context.get("title", "Unknown Track")) + # $cdnum must replace before $track to follow the longest-prefix-first + # rule used throughout this function (no current $c* var collides, but + # ordering matches the web_server.py path-builder for parity). + result = result.replace("$cdnum", cdnum_value) result = result.replace("$track", f"{clean_context.get('track_number', 1):02d}") result = result.replace("$year", str(clean_context.get("year", ""))) @@ -248,6 +262,7 @@ def get_file_path_from_template_raw(template: str, context: dict) -> tuple[str, part = part.replace("$quality", "") part = part.replace("$discnum", "") part = part.replace("$disc", "") + part = part.replace("$cdnum", "") part = re.sub(r"\s*\[\s*\]", "", part) part = re.sub(r"\s*\(\s*\)", "", part) part = re.sub(r"\s*\{\s*\}", "", part) @@ -264,6 +279,9 @@ def get_file_path_from_template_raw(template: str, context: dict) -> tuple[str, filename_base = re.sub(r"\s*\(\s*\)", "", filename_base) filename_base = re.sub(r"\s*\{\s*\}", "", filename_base) filename_base = re.sub(r"\s*-\s*$", "", filename_base) + # Leading dash cleanup — lets $cdnum at the start of a filename + # cleanly disappear on single-disc albums (empty-value case). + filename_base = re.sub(r"^\s*-\s*", "", filename_base) filename_base = re.sub(r"\s+", " ", filename_base).strip() sanitized_folders = [sanitize_filename(part) for part in cleaned_folders] @@ -314,6 +332,7 @@ def get_file_path_from_template(context: dict, template_type: str = "album_path" part = part.replace("$quality", "") part = part.replace("$discnum", "") part = part.replace("$disc", "") + part = part.replace("$cdnum", "") part = re.sub(r"\s*\[\s*\]", "", part) part = re.sub(r"\s*\(\s*\)", "", part) part = re.sub(r"\s*\{\s*\}", "", part) @@ -330,6 +349,9 @@ def get_file_path_from_template(context: dict, template_type: str = "album_path" filename_base = re.sub(r"\s*\(\s*\)", "", filename_base) filename_base = re.sub(r"\s*\{\s*\}", "", filename_base) filename_base = re.sub(r"\s*-\s*$", "", filename_base) + # Leading dash cleanup — lets $cdnum at the start of a filename + # cleanly disappear on single-disc albums (empty-value case). + filename_base = re.sub(r"^\s*-\s*", "", filename_base) filename_base = re.sub(r"\s+", " ", filename_base).strip() sanitized_folders = [sanitize_filename(part) for part in cleaned_folders] @@ -518,8 +540,20 @@ def build_final_path_for_track(context, artist_context, album_info, file_ext): except Exception as _disc_err: logger.warning("[Multi-Disc] Could not resolve total_discs: %s", _disc_err) - album_template = _get_config_manager().get("file_organization.templates.album_path", "") - user_controls_disc = "$disc" in album_template + # Now that total_discs is fully resolved, expose it to the template + # so $cdnum can decide between "CDxx" and an empty string. + template_context["total_discs"] = total_discs + + album_template = _get_config_manager().get("file_organization.templates", {}).get("album_path", "") or "" + # Suppress the auto-injected disc folder when the user already + # encodes the disc in the filename via $disc, $discnum, or $cdnum. + user_controls_disc = ( + "$disc" in album_template + or "$cdnum" in album_template + or "${disc}" in album_template + or "${discnum}" in album_template + or "${cdnum}" in album_template + ) disc_label = _get_config_manager().get("file_organization.disc_label", "Disc") folder_path, filename_base = get_file_path_from_template(template_context, "album_path") diff --git a/tests/imports/test_import_paths.py b/tests/imports/test_import_paths.py index e7194089..899bfec6 100644 --- a/tests/imports/test_import_paths.py +++ b/tests/imports/test_import_paths.py @@ -75,6 +75,72 @@ def test_get_file_path_from_template_raw_handles_quality_and_disc_placeholders(m assert filename == "3 - Song One [FLAC 16bit]" +def test_get_file_path_from_template_raw_substitutes_cdnum_for_multi_disc(monkeypatch): + """$cdnum should expand to 'CDxx' on multi-disc albums (regression). + + Reported by user: filenames had literal '$cdnum' instead of 'CD02' + because `_replace_template_variables` did not handle the placeholder. + """ + monkeypatch.setattr(import_paths, "_get_config_manager", lambda: _Config({})) + + folder_path, filename = import_paths.get_file_path_from_template_raw( + "$artist/$album/$cdnum - $track - $title", + { + "artist": "Artist", + "album": "Album", + "title": "Song", + "track_number": 5, + "disc_number": 2, + "total_discs": 2, + }, + ) + + assert folder_path == os.path.join("Artist", "Album") + assert filename == "CD02 - 05 - Song" + + +def test_get_file_path_from_template_raw_collapses_cdnum_for_single_disc(monkeypatch): + """$cdnum should expand to '' on single-disc albums so it disappears cleanly.""" + monkeypatch.setattr(import_paths, "_get_config_manager", lambda: _Config({})) + + folder_path, filename = import_paths.get_file_path_from_template_raw( + "$artist/$album/$cdnum - $track - $title", + { + "artist": "Artist", + "album": "Album", + "title": "Song", + "track_number": 5, + "disc_number": 1, + "total_discs": 1, + }, + ) + + # No "CD01" prefix; trailing-dash regex collapses the empty placeholder. + assert folder_path == os.path.join("Artist", "Album") + assert filename == "05 - Song" + + +def test_get_file_path_from_template_raw_strips_cdnum_from_folders(monkeypatch): + """Even if user puts $cdnum inside a folder segment, it gets stripped (defensive).""" + monkeypatch.setattr(import_paths, "_get_config_manager", lambda: _Config({})) + + folder_path, filename = import_paths.get_file_path_from_template_raw( + "$artist/$album/$cdnum/$track - $title", + { + "artist": "Artist", + "album": "Album", + "title": "Song", + "track_number": 5, + "disc_number": 1, + "total_discs": 1, + }, + ) + + # Folder containing only $cdnum (which expands to empty for single-disc) gets dropped. + assert folder_path == os.path.join("Artist", "Album") + assert filename == "05 - Song" + + def test_resolve_album_group_upgrades_standard_to_deluxe(): album_naming.clear_album_grouping_cache() @@ -230,3 +296,79 @@ def test_build_final_path_for_track_uses_track_disc_number_without_provider_look assert final_path == str( tmp_path / "Transfer" / "Artist One" / "Artist One - Album One" / "Disc 2" / "04 - Song Two.flac" ) + + +def test_build_final_path_for_track_with_cdnum_template_skips_disc_folder(monkeypatch, tmp_path): + """When the user template encodes the disc via $cdnum, the auto disc folder must not be added. + + Reported by user: multi-disc albums got both a "CDxx" label in the + filename AND a redundant "Disc N" folder. The auto-folder should + suppress when the template already encodes the disc. + """ + config = _Config( + { + "soulseek.transfer_path": str(tmp_path / "Transfer"), + "file_organization.enabled": True, + "file_organization.templates": { + "album_path": "$albumartist/$albumartist - $album/$cdnum - $track - $title", + "single_path": "$artist/$artist - $title", + }, + "file_organization.collab_artist_mode": "first", + "file_organization.disc_label": "Disc", + } + ) + monkeypatch.setattr(import_paths, "_get_config_manager", lambda: config) + monkeypatch.setattr( + import_paths, + "_get_album_tracks_for_source", + lambda source, album_id: None, + ) + + context = { + "artist": {"name": "Artist One"}, + "album": { + "name": "Album One", + "id": "album-1", + "release_date": "2026-01-01", + "total_tracks": 24, + "total_discs": 2, + "album_type": "album", + "artists": [{"name": "Artist One"}], + }, + "track_info": { + "name": "Song Two", + "id": "track-2", + "track_number": 4, + "disc_number": 2, + "artists": [{"name": "Artist One"}], + }, + "original_search_result": { + "title": "Song Two", + "clean_title": "Song Two", + "clean_album": "Album One", + "clean_artist": "Artist One", + "artists": [{"name": "Artist One"}], + }, + "source": "deezer", + "is_album_download": False, + } + + final_path, created = import_paths.build_final_path_for_track( + context, + {"name": "Artist One"}, + { + "is_album": True, + "album_name": "Album One", + "track_number": 4, + "disc_number": 2, + }, + ".flac", + ) + + assert created is True + # Filename has the CD02 label; NO "Disc 2" folder injected. + assert final_path == str( + tmp_path / "Transfer" / "Artist One" / "Artist One - Album One" / "CD02 - 04 - Song Two.flac" + ) + # Verify the disc folder was not created either. + assert not (tmp_path / "Transfer" / "Artist One" / "Artist One - Album One" / "Disc 2").exists()