Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .forgeos/intent/findaway-dual-chapter-numbering-app-reconcile.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Intent — Findaway dual chapter-numbering: app reconcile (PR2)

Critical path: `Palace/Audiobooks/`. Pairs with toolkit PR
ThePalaceProject/ios-audiobooktoolkit#184 (the TOC-collapse fix).

## Claims
1. Bumps the `ios-audiobooktoolkit` submodule pointer `28de55a` → `51d291c` (the
TOC-collapse fix: one chapter per physical track for oversubdivided/dense
manifests).
2. Makes `AudiobookSessionManager.normalizedChapters(for:)` a **passthrough**
(`return toc.toc`) — the toolkit now owns TOC collapse, so the app consumes the
already-collapsed list. Eliminates the second collapse implementation that
diverged from the toolkit (the Findaway "Dune" dual-numbering bug).
3. Adds `PalaceTests/Audiobook/FindawaySavedVsPlayedTests.swift` + the
`dune_oversubdivided_manifest.json` fixture: asserts the saved-vs-played
invariant (a bookmark taken on physical track findaway:1:3 saves to 1:3, not the
device-log's 1:4, and round-trips back to 1:3).

## Anti-claims
- Does NOT bump the build number (CURRENT_PROJECT_VERSION) — no new TestFlight
build (release gate active).
- Does NOT change the player/engine, the position-save format, the Manifest
decoder, or `TrackPosition`/`toAudioBookmark`.
- Does NOT delete `ChapterTOCNormalizer` / `normalizedChaptersCount` — they remain
as the unit-tested threshold spec (now implemented in the toolkit); the
production collapse path is the only thing removed from the app.

## Files in scope
- `ios-audiobooktoolkit` (gitlink bump)
- `Palace/Audiobooks/AudiobookSessionManager.swift` (passthrough)
- `PalaceTests/Audiobook/FindawaySavedVsPlayedTests.swift` (new)
- `PalaceTests/dune_oversubdivided_manifest.json` (new fixture)
- `Palace.xcodeproj/project.pbxproj` (PalaceTests target wiring)
8 changes: 8 additions & 0 deletions Palace.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
0B461FFD655EC35ED6C3990A /* OPDSFeedCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = ACE8017D2ECF7E0FEBFAAB1A /* OPDSFeedCache.swift */; };
0B55EFB324FAF34D779EB878 /* StreamingReaderPresentationContractTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3A213D524079F39BA952025A /* StreamingReaderPresentationContractTests.swift */; };
0B72C5306C699DBCB586DBE7 /* MyBooksDownloadCenterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 317C8F0150303A9AC53BC44F /* MyBooksDownloadCenterTests.swift */; };
0B9C4F9CCCF2321228F96954 /* dune_oversubdivided_manifest.json in Resources */ = {isa = PBXBuildFile; fileRef = 2630448EB64E91E2EADDF594 /* dune_oversubdivided_manifest.json */; };
0C26C51E4B1F06A4C02FBB65 /* TPPReauthenticator+Reauthenticating.swift in Sources */ = {isa = PBXBuildFile; fileRef = 540375EB4B496BAA15E4F6F4 /* TPPReauthenticator+Reauthenticating.swift */; };
0C6A411969E83D1D9A8BD2EA /* BorrowOperationAuthCoordinatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E5CDE8C912B8AD7B20AF282E /* BorrowOperationAuthCoordinatorTests.swift */; };
0CE51471A7664B7995B5C8A0 /* AudiobookDataManagerSyncTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FE7BD91AF3E14990B745A0AA /* AudiobookDataManagerSyncTests.swift */; };
Expand Down Expand Up @@ -830,6 +831,7 @@
9309887A8360976E65C6AE04 /* StatsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5431B832D502714505B6EAF5 /* StatsView.swift */; };
9325024229965F9F9996F9BA /* ReadiumPDFViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5472258280B72752C01480DF /* ReadiumPDFViewController.swift */; };
935781EA3FD5B824D9A52F21 /* OfflineQueueDetailView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 634E8A7700449B3157A0E943 /* OfflineQueueDetailView.swift */; };
935B58EA66D6CDC630490B7E /* FindawaySavedVsPlayedTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F774A40BD0FCA32D809EFDA7 /* FindawaySavedVsPlayedTests.swift */; };
937A08D0914606B4C64933F3 /* UIAlertCACommitGuardTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45EB45618938C811D051DEB7 /* UIAlertCACommitGuardTests.swift */; };
93C86ECD853FB5864CE8B5FF /* AccountTestSeeder.swift in Sources */ = {isa = PBXBuildFile; fileRef = E51335D9B0719C849AA9B591 /* AccountTestSeeder.swift */; };
953B933A06DA6AF6ABAFB7EC /* BadgeDetailView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6837ACC21376FE419332599B /* BadgeDetailView.swift */; };
Expand Down Expand Up @@ -2191,6 +2193,7 @@
25174690C33556366CFFE575 /* TPPBookDRMProtectedTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = TPPBookDRMProtectedTests.swift; sourceTree = "<group>"; };
260C03DB276DAE8ED383D25A /* AudiobookSessionPresenter.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AudiobookSessionPresenter.swift; sourceTree = "<group>"; };
262879F496344EE8F262446A /* AppInfrastructureCoverageTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AppInfrastructureCoverageTests.swift; sourceTree = "<group>"; };
2630448EB64E91E2EADDF594 /* dune_oversubdivided_manifest.json */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.json; path = dune_oversubdivided_manifest.json; sourceTree = "<group>"; };
2634B33D2AE74BF5948971BA /* CarPlayTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CarPlayTests.swift; sourceTree = "<group>"; };
26AAA6A2C834984A7D21A854 /* AudiobookVendorAdapter.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AudiobookVendorAdapter.swift; sourceTree = "<group>"; };
272E56401C680E7771557AC3 /* RemoteFeatureFlags+PalaceCatalog.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = "RemoteFeatureFlags+PalaceCatalog.swift"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3209,6 +3212,7 @@
F6C56509A03466E0531DBC27 /* TPPAccountAuthState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TPPAccountAuthState.swift; sourceTree = "<group>"; };
F714C5D09735CC88BE76576D /* PlaybackReadinessGate.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = PlaybackReadinessGate.swift; sourceTree = "<group>"; };
F75B6896C9742C07208D935E /* MockVisualNavigator.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = MockVisualNavigator.swift; sourceTree = "<group>"; };
F774A40BD0FCA32D809EFDA7 /* FindawaySavedVsPlayedTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = FindawaySavedVsPlayedTests.swift; sourceTree = "<group>"; };
F8091A2B3C4D5E6F70819213 /* TPPPreferredAuthSelectionTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = TPPPreferredAuthSelectionTests.swift; sourceTree = "<group>"; };
F820025C44C02D27C13B48B9 /* SAMLCookieSyncTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = SAMLCookieSyncTests.swift; sourceTree = "<group>"; };
F84A9BADA3014DCF9808C7DC /* NSErrorAdditionsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = NSErrorAdditionsTests.swift; path = ErrorHandling/NSErrorAdditionsTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3824,6 +3828,7 @@
2D831302F218AC0EE46308A0 /* AudiobookLoaderDispatchTests.swift */,
7EA3C09A7973CF58C57F23C0 /* AudiobookLoaderOPDSShapeMatrixTests.swift */,
73D1CD4CDD735A3008ACCD5F /* AudiobookBookmarkBusinessLogicPositionWriteTests.swift */,
F774A40BD0FCA32D809EFDA7 /* FindawaySavedVsPlayedTests.swift */,
);
path = Audiobook;
sourceTree = "<group>";
Expand Down Expand Up @@ -5253,6 +5258,7 @@
2D8790A220129AC400E2763F /* NYPLOPDSAcquisitionPathEntry.xml */,
E72B614A28AD7612008CFEBB /* TPPOPDSAcquisitionPathEntryWithSampleLink.xml */,
73A794C62549095700C59CC1 /* NYPLOPDSAcquisitionPathEntryMinimal.xml */,
2630448EB64E91E2EADDF594 /* dune_oversubdivided_manifest.json */,
);
name = "Supporting Files";
sourceTree = "<group>";
Expand Down Expand Up @@ -6640,6 +6646,7 @@
732F4748260AD76E00E2CB64 /* NYPLOPDSAcquisitionPathEntry.xml in Resources */,
73A794C72549098700C59CC1 /* NYPLOPDSAcquisitionPathEntryMinimal.xml in Resources */,
B05017333F74A15B3995290E /* baselines in Resources */,
0B9C4F9CCCF2321228F96954 /* dune_oversubdivided_manifest.json in Resources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down Expand Up @@ -7391,6 +7398,7 @@
63F7E41142B47B5051055271 /* RuntimeQuiescenceLintTests.swift in Sources */,
36D5376F01972FB0278F2CB9 /* AudioSessionActivatorTests.swift in Sources */,
F58124F59C1F78CADB673820 /* TriageBotKeyAdminTests.swift in Sources */,
935B58EA66D6CDC630490B7E /* FindawaySavedVsPlayedTests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
46 changes: 16 additions & 30 deletions Palace/Audiobooks/AudiobookSessionManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1277,39 +1277,25 @@ public final class AudiobookSessionManager: ObservableObject {

// MARK: - Chapter TOC normalization

/// Returns the chapter list that should drive `currentChapters`. When the
/// raw TOC is oversubdivided relative to the actual track count (>1.5x),
/// collapses adjacent same-track entries to keep the UI showing chapters
/// (one per track) instead of sections/paragraphs. See
/// `ChapterTOCNormalizer` for the threshold rationale.
///
/// Static + pure so the decision is unit-testable without a real
/// `AudiobookTableOfContents` — wrapper `normalizedChaptersCount(
/// tocCount:trackCount:)` provides a primitive entry point that tests
/// can hit without constructing toolkit types.
/// Passthrough. TOC collapse now lives entirely in the toolkit's
/// `AudiobookTableOfContents` (one chapter per physical track for
/// oversubdivided / dense manifests), so the app consumes the already-collapsed
/// list directly. This eliminates the SECOND collapse implementation that
/// diverged from the toolkit and produced the Findaway "Dune" dual chapter-
/// numbering (the toolkit used the uncollapsed list for currentChapter /
/// NowPlaying / saved-position while the app displayed the collapsed one).
/// Retained as a named seam for the bind site (`currentChapters`) + tests.
static func normalizedChapters(for toc: AudiobookTableOfContents) -> [Chapter] {
let chapters = toc.toc
let trackCount = toc.tracks.tracks.count
if !ChapterTOCNormalizer.isOversubdivided(tocCount: chapters.count, expectedChapterCount: trackCount) {
return chapters
}
// Collapse: keep the FIRST chapter object encountered per track key.
// This preserves the natural reading order while dropping subsections.
var seenKeys = Set<String>()
var collapsed: [Chapter] = []
collapsed.reserveCapacity(trackCount)
for chapter in chapters {
let key = chapter.position.track.key
if seenKeys.insert(key).inserted {
collapsed.append(chapter)
}
}
return collapsed
toc.toc
}

/// Primitive-typed mirror of `normalizedChapters(for:)` for unit-test
/// use. Returns the expected output count given a TOC count + track
/// count, without needing the toolkit's Chapter type.
/// Test-only spec for the 1.5x oversubdivision THRESHOLD (one chapter per
/// physical track when `tocCount > trackCount * 1.5`). NOTE: this is no longer
/// the production collapse seam — `normalizedChapters(for:)` is now a passthrough
/// and the collapse is implemented in the toolkit's `AudiobookTableOfContents`
/// (`isOversubdivided` + keep-first). This primitive is retained only to keep the
/// threshold math pinned by unit tests without constructing toolkit types; it
/// does not drive any production code path.
static func normalizedChaptersCount(tocCount: Int, trackCount: Int) -> Int {
if !ChapterTOCNormalizer.isOversubdivided(tocCount: tocCount, expectedChapterCount: trackCount) {
return tocCount
Expand Down
67 changes: 67 additions & 0 deletions PalaceTests/Audiobook/FindawaySavedVsPlayedTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
//
// FindawaySavedVsPlayedTests.swift
// PalaceTests
//
// App-side end-to-end for the 3.2.0 Findaway dual chapter-numbering regression
// ("Dune", Findaway id 32884). Device log: saved key findaway:1:4 while the engine
// played (1,3); "Part 1 Chapter 2/3/4" all resolved onto physical file 1:3. With
// the toolkit's TOC collapse, the chapter SHOWN == the chapter SAVED == the played
// track, so a saved bookmark round-trips back to the file it was taken on.
//
// GREEN-ONLY (by construction): this asserts the POST-FIX invariant
// (saved == played) rather than red-firsting the bug at the app layer. The
// collapse that produces the invariant lives in the ios-audiobooktoolkit submodule
// (already bumped to the fix here), so an app-layer red-first would require
// reverting the submodule. The BUG DIRECTION -- pre-fix the uncollapsed list
// resolved several chapters onto one physical key, so saved 1:4 != played 1:3 -- is
// proven red-first in the toolkit suite (FindawayOversubdividedTOCTests: toc 5->3,
// three indices on findaway:1:3). This app test is the consumer-side smoke that the
// invariant holds through toAudioBookmark + the position round-trip.
//

import XCTest
@testable import Palace
@testable import PalaceAudiobookToolkit

final class FindawaySavedVsPlayedTests: XCTestCase {
private let testID = "DuneSavedVsPlayed"
private let playedKey = "urn:org.thepalaceproject:findaway:1:3"

private func loadDune() throws -> (AudiobookTableOfContents, Tracks) {
let manifest = try Manifest.from(
jsonFileName: "dune_oversubdivided_manifest",
bundle: Bundle(for: type(of: self)))
let tracks = Tracks(manifest: manifest, audiobookID: testID, token: nil)
return (AudiobookTableOfContents(manifest: manifest, tracks: tracks), tracks)
}

/// SAVED == PLAYED: a TrackPosition physically on the oversubdivided file
/// (findaway:1:3 @34.757) must save to that SAME played track — not a neighbor
/// (the device log's saved 1:4 ≠ played 1:3) — and round-trip back to it.
func testDuneOversubdivided_savedBookmarkKeyEqualsPlayedTrack() throws {
let (toc, tracks) = try loadDune()
guard let played = tracks.track(forKey: playedKey) else {
return XCTFail("fixture must contain physical track \(playedKey)")
}
let position = TrackPosition(track: played, timestamp: 34.757, tracks: tracks)

// The saved bookmark records the PLAYED track's key (not 1:4).
let bookmark = position.toAudioBookmark()
XCTAssertEqual(
bookmark.readingOrderItem, playedKey,
"Saved bookmark key must equal the played track \(playedKey); device log showed saved 1:4 ≠ played 1:3.")

// Round-trip: restoring the bookmark lands back on the SAME physical track.
let restored = TrackPosition(audioBookmark: bookmark, toc: toc.toc, tracks: tracks)
XCTAssertEqual(
restored?.track.key, playedKey,
"Restored position must resolve to the played track (saved == played).")

// The chapter SHOWN for that position is the single collapsed chapter on 1:3.
let shown = try toc.chapter(forPosition: position)
XCTAssertEqual(shown.position.track.key, playedKey, "Shown/saved chapter must be on the played track.")
XCTAssertEqual(
toc.toc.filter { $0.position.track.key == playedKey }.count, 1,
"Exactly one collapsed chapter backs the played file (no dual numbering).")
}
}
32 changes: 32 additions & 0 deletions PalaceTests/dune_oversubdivided_manifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"metadata": {
"author": ["Herbert, Frank"],
"@type": "http://bib.schema.org/Audiobook",
"title": "Dune (synthetic oversubdivided Findaway repro)",
"encrypted": {
"findaway:licenseId": "00000000000000000000repro",
"findaway:sessionKey": "00000000-0000-0000-0000-000000repro",
"findaway:checkoutId": "00000000000000000000chkout",
"findaway:fulfillmentId": "32884",
"findaway:accountId": "REPRO-acct",
"scheme": "http://librarysimplified.org/terms/drm/scheme/FAE"
},
"identifier": "urn:librarysimplified.org/terms/id/Findaway%20ID/32884",
"language": "en",
"duration": 4943.924
},
"links": [
{ "rel": "cover", "href": "https://example.org/cover/32884" }
],
"@context": [
"http://readium.org/webpub/default.jsonld",
{ "findaway": "http://librarysimplified.org/terms/third-parties/findaway.com/" }
],
"readingOrder": [
{"findaway:part": 1, "findaway:sequence": 1, "title": "Chapter 1", "type": "audio/mpeg", "duration": 600.0},
{"findaway:part": 1, "findaway:sequence": 3, "title": "Chapter 2", "type": "audio/mpeg", "duration": 1257.874},
{"findaway:part": 1, "findaway:sequence": 3, "title": "Chapter 3", "type": "audio/mpeg", "duration": 998.243},
{"findaway:part": 1, "findaway:sequence": 3, "title": "Chapter 4", "type": "audio/mpeg", "duration": 1387.807},
{"findaway:part": 1, "findaway:sequence": 4, "title": "Chapter 5", "type": "audio/mpeg", "duration": 700.0}
]
}
Loading