diff --git a/listenarr.application/Common/FileNamingService.Helpers.cs b/listenarr.application/Common/FileNamingService.Helpers.cs index 327616b52..a0156d7ad 100644 --- a/listenarr.application/Common/FileNamingService.Helpers.cs +++ b/listenarr.application/Common/FileNamingService.Helpers.cs @@ -85,7 +85,11 @@ private Dictionary BuildVariables(AudioMetadata metadata) { "Publisher", string.IsNullOrWhiteSpace(metadata.Publisher) ? string.Empty : SanitizePathComponent(metadata.Publisher) }, { "Language", string.IsNullOrWhiteSpace(metadata.Language) ? string.Empty : SanitizePathComponent(metadata.Language) }, { "Asin", string.IsNullOrWhiteSpace(metadata.Asin) ? string.Empty : SanitizePathComponent(metadata.Asin) }, - { "SeriesNumber", FirstNonEmpty(metadata.SeriesPosition?.ToString(CultureInfo.InvariantCulture), metadata.TrackNumber?.ToString()) }, + // Prefer the position exactly as the source gave it. A non-numeric but real + // position (an omnibus at "1-4") does not survive the decimal parse, and + // falling through to TrackNumber here would write a track number into the + // filename as if it were the series number. + { "SeriesNumber", FirstNonEmpty(metadata.SeriesPositionRaw, metadata.SeriesPosition?.ToString(CultureInfo.InvariantCulture), metadata.TrackNumber?.ToString()) }, { "Year", FirstNonEmpty(metadata.Year?.ToString()) }, { "Quality", FirstNonEmpty(metadata.BitRate.HasValue ? metadata.BitRate + "kbps" : null, metadata.Format) }, { "DiskNumber", metadata.DiscNumber?.ToString() ?? string.Empty }, diff --git a/listenarr.application/Downloads/Import/DownloadImportService.cs b/listenarr.application/Downloads/Import/DownloadImportService.cs index d6cc98272..a45ac20db 100644 --- a/listenarr.application/Downloads/Import/DownloadImportService.cs +++ b/listenarr.application/Downloads/Import/DownloadImportService.cs @@ -15,6 +15,7 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . */ +using System.Globalization; using Listenarr.Domain.Common; using Microsoft.Extensions.Logging; @@ -230,7 +231,10 @@ public async Task> ImportDownloadFilesAsync( { "Publisher", string.IsNullOrWhiteSpace(namingMetadata.Publisher) ? string.Empty : namingMetadata.Publisher }, { "Language", string.IsNullOrWhiteSpace(namingMetadata.Language) ? string.Empty : namingMetadata.Language }, { "Asin", string.IsNullOrWhiteSpace(namingMetadata.Asin) ? string.Empty : namingMetadata.Asin }, - { "SeriesNumber", namingMetadata.SeriesPosition?.ToString() ?? effectiveChapterNumber?.ToString() ?? string.Empty }, + // As in FileNamingService: prefer the raw position, and format the + // decimal with InvariantCulture -- ToString() under the server's + // culture would put a comma into the filename. + { "SeriesNumber", FirstNonEmpty(namingMetadata.SeriesPositionRaw, namingMetadata.SeriesPosition?.ToString(CultureInfo.InvariantCulture), effectiveChapterNumber?.ToString()) }, { "Year", namingMetadata.Year?.ToString() ?? string.Empty }, { "Quality", (namingMetadata.BitRate.HasValue ? $"{namingMetadata.BitRate}kbps" : null) ?? namingMetadata.Format ?? string.Empty }, { "DiskNumber", effectiveDiskNumber?.ToString() ?? string.Empty }, @@ -368,9 +372,11 @@ private static AudioMetadata BuildNamingMetadata(Audiobook? audiobook, AudioMeta Language = FirstNonEmpty(audiobook.Language, extractedMetadata?.Language), Asin = FirstNonEmpty(audiobook.Asin, extractedMetadata?.Asin), Series = FirstNonEmpty(audiobook.Series, extractedMetadata?.Series), - SeriesPosition = !string.IsNullOrWhiteSpace(audiobook.SeriesNumber) && decimal.TryParse(audiobook.SeriesNumber, out var sp) - ? sp - : (extractedMetadata?.SeriesPosition), + SeriesPosition = !string.IsNullOrWhiteSpace(audiobook.SeriesNumber) + && decimal.TryParse(audiobook.SeriesNumber, NumberStyles.Number, CultureInfo.InvariantCulture, out var sp) + ? sp + : (extractedMetadata?.SeriesPosition), + SeriesPositionRaw = FirstNonEmpty(audiobook.SeriesNumber, extractedMetadata?.SeriesPositionRaw), Year = !string.IsNullOrWhiteSpace(audiobook.PublishYear) && int.TryParse(audiobook.PublishYear, out var year) ? year : extractedMetadata?.Year, diff --git a/listenarr.domain/Audiobooks/AudioMetadata.cs b/listenarr.domain/Audiobooks/AudioMetadata.cs index 3a8ccfc4e..826a8f02b 100644 --- a/listenarr.domain/Audiobooks/AudioMetadata.cs +++ b/listenarr.domain/Audiobooks/AudioMetadata.cs @@ -50,6 +50,22 @@ public class AudioMetadata public string? Language { get; set; } public string? Series { get; set; } public decimal? SeriesPosition { get; set; } + + /// + /// The series position exactly as the metadata source gave it. + /// + /// Audible/Audnexus report a position as a string, and it is not always a number: + /// an omnibus can sit at "1-4", a prequel at "0", a novella at "1.5". Those values + /// are meaningful, but they do not all fit in , so a + /// position that fails to parse would otherwise be indistinguishable from a book + /// that has no series position at all. + /// + /// + /// Naming prefers this over so that a real position is + /// never silently replaced by a track number. + /// + /// + public string? SeriesPositionRaw { get; set; } public byte[]? CoverArt { get; set; } public string? CoverArtUrl { get; set; } public Dictionary AdditionalData { get; set; } = []; @@ -75,6 +91,8 @@ public void Update(AudioMetadata value) if (!SeriesPosition.HasValue && value.SeriesPosition.HasValue) SeriesPosition = value.SeriesPosition; + if (string.IsNullOrWhiteSpace(SeriesPositionRaw) && !string.IsNullOrWhiteSpace(value.SeriesPositionRaw)) + SeriesPositionRaw = value.SeriesPositionRaw; if (!TrackNumber.HasValue && value.TrackNumber.HasValue) TrackNumber = value.TrackNumber; if (!DiscNumber.HasValue && value.DiscNumber.HasValue) diff --git a/listenarr.domain/Audiobooks/Audiobook.cs b/listenarr.domain/Audiobooks/Audiobook.cs index 54c3796f7..36fc93fda 100644 --- a/listenarr.domain/Audiobooks/Audiobook.cs +++ b/listenarr.domain/Audiobooks/Audiobook.cs @@ -17,6 +17,7 @@ */ using System.ComponentModel.DataAnnotations; +using System.Globalization; using Listenarr.Domain.Common; namespace Listenarr.Domain.Audiobooks @@ -104,8 +105,17 @@ public AudioMetadata CreateBasicAudioMetadata() Series = Series, // Prefer audiobook's publish year when available Year = int.TryParse(PublishYear, out var py) ? py : (int?)null, - // Series position / number - SeriesPosition = !string.IsNullOrWhiteSpace(SeriesNumber) && decimal.TryParse(SeriesNumber, out var sp) ? sp : (decimal?)null, + // Series position / number. + // Parsed with InvariantCulture: the source value always uses '.' as the + // decimal separator, so parsing under the server's culture would read a + // position of "1.5" as 15 wherever '.' is the group separator. + // SeriesPositionRaw keeps the original either way -- a position such as + // "1-4" (an omnibus) is real, but is not a decimal. + SeriesPosition = !string.IsNullOrWhiteSpace(SeriesNumber) + && decimal.TryParse(SeriesNumber, NumberStyles.Number, CultureInfo.InvariantCulture, out var sp) + ? sp + : (decimal?)null, + SeriesPositionRaw = !string.IsNullOrWhiteSpace(SeriesNumber) ? SeriesNumber.Trim() : null, // Quality string from audiobook record // Map into Bitrate/Format heuristically if useful; for now store textual quality // We'll put it into AdditionalData so FileNamingService can use Format/Bitrate/Quality diff --git a/tests/Features/Domain/Audiobooks/SeriesPositionReproTests.cs b/tests/Features/Domain/Audiobooks/SeriesPositionReproTests.cs new file mode 100644 index 000000000..efc05c573 --- /dev/null +++ b/tests/Features/Domain/Audiobooks/SeriesPositionReproTests.cs @@ -0,0 +1,159 @@ +/* + * Listenarr - Audiobook Management System + * Copyright (C) 2024-2026 Listenarr Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +using System.Globalization; +using Listenarr.Application.Common; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Listenarr.Tests.Features.Domain.Audiobooks +{ + /// + /// Audible/Audnexus report a series position as a STRING, and it is not always a number. + /// Real, live examples from the catalogue: + /// + /// "The Father Brown Collection: Books 1-4" -> position "1-4" (one ASIN, four books) + /// "The Thirty-Nine Steps" -> position "1-2" (bundles its sequel) + /// "She and Allan" -> position "0" (prequel slot) + /// a novella between two books -> position "1.5" + /// + /// Two defects followed from squeezing that string through a decimal: + /// + /// 1. The parse used the server's culture. Where '.' is the group separator (de-DE), + /// "1.5" parses as 15; under fr-FR it does not parse at all. + /// 2. A position that does not parse became null, which is indistinguishable from a + /// book with NO series position -- so naming fell through to the track number and + /// wrote it into the filename as if it were the series number. + /// + public class SeriesPositionReproTests + { + private static Audiobook Book(string? seriesNumber) => new() + { + Title = "Test", + Series = "Test Series", + SeriesNumber = seriesNumber, + }; + + private static FileNamingService Naming() + { + var config = new Mock(); + var logger = new Mock>(); + return new FileNamingService(config.Object, logger.Object); + } + + // --------------------------------------------------------------- + // 1. Culture: the source always uses '.', so parsing must be invariant. + // --------------------------------------------------------------- + + [Theory] + [InlineData("en-US")] + [InlineData("de-DE")] // '.' is the GROUP separator here -- "1.5" once parsed as 15 + [InlineData("fr-FR")] // "1.5" once failed to parse at all + public void DecimalPosition_SurvivesAnyServerCulture(string culture) + { + var original = CultureInfo.CurrentCulture; + try + { + CultureInfo.CurrentCulture = new CultureInfo(culture); + var metadata = Book("1.5").CreateBasicAudioMetadata(); + + Assert.Equal(1.5m, metadata.SeriesPosition); + } + finally + { + CultureInfo.CurrentCulture = original; + } + } + + [Fact] + public void DecimalPosition_IsWrittenInvariantly_NotWithALocalDecimalComma() + { + var original = CultureInfo.CurrentCulture; + try + { + CultureInfo.CurrentCulture = new CultureInfo("de-DE"); + var metadata = Book("1.5").CreateBasicAudioMetadata(); + + var name = Naming().ApplyNamingPattern("{SeriesNumber}", metadata, treatAsFilename: true); + + // Not "1,5" -- a comma in a filename is a locale leaking onto disk. + Assert.Equal("1.5", name); + } + finally + { + CultureInfo.CurrentCulture = original; + } + } + + // --------------------------------------------------------------- + // 2. A real but non-numeric position must not be lost. + // --------------------------------------------------------------- + + [Theory] + [InlineData("1-4")] // The Father Brown Collection: Books 1-4 + [InlineData("1-2")] // The Thirty-Nine Steps (bundles Greenmantle) + public void RangePosition_IsPreserved_EvenThoughItIsNotADecimal(string position) + { + var metadata = Book(position).CreateBasicAudioMetadata(); + + // decimal? genuinely cannot hold "1-4", and should not try. + Assert.Null(metadata.SeriesPosition); + + // But the value is real and must survive. + Assert.Equal(position, metadata.SeriesPositionRaw); + } + + [Theory] + [InlineData("1-4")] + [InlineData("1-2")] + public void RangePosition_ReachesTheFilename_AndIsNotReplacedByTheTrackNumber(string position) + { + var metadata = Book(position).CreateBasicAudioMetadata(); + metadata.TrackNumber = 7; // the value that used to be written instead + + var name = Naming().ApplyNamingPattern("{SeriesNumber}", metadata, treatAsFilename: true); + + Assert.Equal(position, name); + Assert.NotEqual("7", name); + } + + [Fact] + public void AbsentPosition_StillFallsBackToTheTrackNumber() + { + // The fallback itself is deliberate and must be preserved: a book with no series + // position at all should still get the track number. The bug was that a REAL + // position was being treated as an absent one. + var metadata = Book(null).CreateBasicAudioMetadata(); + metadata.TrackNumber = 7; + + var name = Naming().ApplyNamingPattern("{SeriesNumber}", metadata, treatAsFilename: true); + + Assert.Equal("7", name); + } + + [Fact] + public void ZeroPosition_Survives() + { + // She and Allan sits at position "0" of the Ayesha series. + var metadata = Book("0").CreateBasicAudioMetadata(); + + Assert.Equal(0m, metadata.SeriesPosition); + Assert.Equal("0", metadata.SeriesPositionRaw); + } + } +}