fix(naming): don't lose a series position that isn't a plain number - #763
fix(naming): don't lose a series position that isn't a plain number#763m4bard wants to merge 1 commit into
Conversation
|
Rebased this onto #717 locally. Half of it needs attention sooner than the rebase does. On unmodified #717 head, under the invariant culture a container runs with by default, a book whose series position is "1-4" gets the track number written into its filename: No locale involved. The other half is the culture-dependent parse, which only shows up with Both come back in the new file:
SeriesPosition = !string.IsNullOrWhiteSpace(audiobook.SeriesNumber)
&& decimal.TryParse(audiobook.SeriesNumber, out var seriesPosition)
? seriesPosition
: extractedMetadata?.SeriesPosition,
The port comes to five sites against four on canary. Splitting
The test needed adapting too. I ran the full suite on 379bb9d with and without the port and diffed the failing tests. Identical set either way, so the port adds none. I am not claiming a green suite on your branch. It is one commit on top of 379bb9d if you want to cherry-pick it rather than wait for the rebase: |
0ba2f4e to
b9c19b5
Compare
|
Rebased onto canary Before rebasing I checked whether the fix still lands where the value is used, since a clean merge only says no lines collide. There are five places that put a
Three of them already carry the position as the string it arrived as. The two that round-trip it through a Full suite on the rebased branch: 3,039 passed, 0 failed, 125 skipped, against a 3,029 baseline. Removing just the raw-position preference in |
b9c19b5 to
b5ad3d9
Compare
Ported onto Listenarrs#717, where BuildNamingMetadata has moved into DownloadImportService.Naming.cs and the culture-dependent parse is reintroduced there. A position arrives from Audnexus as a string and is not always a decimal: an omnibus sits at "1-4", a prequel at "0", a novella at "1.5". Squeezing it through decimal.TryParse loses the ones that do not parse, and naming then falls through to the track number and writes that into the filename as if it were the series number. The parse also used the server's culture, so "1.5" read as 15 wherever '.' is the group separator. SeriesPositionRaw keeps the original string, naming prefers it, and the remaining decimal parses are pinned to InvariantCulture. On the import side the {SeriesNumber} token is built by a small helper in DownloadImportService.Naming.cs rather than written inline. DownloadImportService.cs is already at the 500 line architecture cap, and the naming partial is where the rest of this logic lives.
b5ad3d9 to
ce39124
Compare
Summary
Fixes #764.
Audnexus reports a series position as a string, and it is not always a number. Squeezing it through a
decimallost it in two ways, and the loss reached the filename.Real positions from the catalogue (public domain, verifiable against
api.audnex.us):positionB0F84DFZ66"1-4"— one ASIN, four booksB002V1PLZK"1-2"— the product also ships GreenmantleB00CQ5WAXW"0"— a prequel slot"1.5"The parse used the server's culture.
decimal.TryParsewas called with noCultureInfo, whileFileNamingServiceformatted the result back withInvariantCulture. The source always uses.as the decimal separator, so where.is the group separator the number changed:A position that didn't parse was silently discarded. It became
null— indistinguishable from a book with no position — so naming fell through to the track number and wrote that into the filename instead. No error, nothing in the log.Changes
Added
AudioMetadata.SeriesPositionRaw— the series position exactly as the metadata source gave it.SeriesPosition(decimal?) is kept for callers that sort or compare, but it cannot hold"1-4"and no longer pretends to.SeriesPositionReproTests— 10 cases covering culture handling and non-numeric positions.Changed
Audiobook.CreateBasicAudioMetadataandDownloadImportServicenow parse the position withInvariantCulture.FileNamingService.Helpers,DownloadImportService) preferSeriesPositionRaw, then the invariant-formatted decimal, then the track number.DownloadImportServicepreviously formatted with a bare.ToString(), which would write1,5— with a comma — into a filename.Fixed
Testing
SeriesPositionReproTests(10 new cases):en-US,de-DEandfr-FR"1-4"and"1-2"are preserved and reach the filename rather than being replaced by the track number"0"survivesdotnet test: 1199 passing, 0 failing (1189 existing + 10 new). No existing test was changed.Notes
The fall back to the track number is deliberate and is preserved — a book with genuinely no series position still gets it. The bug was that a real position was being treated as an absent one.
Apologies for opening this before the issue; #764 has the write-up. I've read
CONTRIBUTING.mdproperly now.Out of scope but worth flagging while it's in view: a book can hold two series memberships at once. Audnexus gives She and Allan (
B00CQ5WAXW)seriesPrimary= Ayesha #0 andseriesSecondary= Allan Quatermain #7.MetadataConvertersreads both, butAudiobookSeriesMembershipis never constructed outside tests, so neither is persisted. Happy to open that separately if useful.