Make Streamable and Resource protocols Sendable #801
Conversation
There was a problem hiding this comment.
Pull request overview
This PR moves core resource streaming APIs toward Swift strict-concurrency compatibility by making streaming/resource protocols and related callbacks Sendable, and by refactoring several resource wrappers to avoid shared mutable captures.
Changes:
- Adds
Sendablerequirements toStreamable,Resource-related callbacks,PositionsService, and HTML font declarations. - Reworks resource transformations and buffering/tail caching to use closure composition and
Mutex-protected accumulators. - Updates LCP decryption and EPUB CSS/font serving paths to reduce concurrency diagnostics.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
Tests/SharedTests/Publication/Services/Positions/PositionsServiceTests.swift |
Converts the test positions service from struct to class. |
Sources/Shared/Toolkit/ZIP/ZIPFoundation/ZIPFoundationContainer.swift |
Updates ZIPFoundation resource streaming callback to @Sendable. |
Sources/Shared/Toolkit/HTTP/HTTPResource.swift |
Updates HTTP resource streaming callback to @Sendable. |
Sources/Shared/Toolkit/File/FileResource.swift |
Updates file resource streaming callback to @Sendable. |
Sources/Shared/Toolkit/Data/Streamable.swift |
Makes Streamable Sendable and protects local read accumulation with Mutex. |
Sources/Shared/Toolkit/Data/Resource/TransformingResource.swift |
Refactors transforming resources from subclassing to closure-based composition. |
Sources/Shared/Toolkit/Data/Resource/TailCachingResource.swift |
Updates streaming callback and protects tail accumulation with Mutex. |
Sources/Shared/Toolkit/Data/Resource/FailureResource.swift |
Updates failure resource streaming callback to @Sendable. |
Sources/Shared/Toolkit/Data/Resource/DataResource.swift |
Makes data factories and streaming callbacks @Sendable. |
Sources/Shared/Toolkit/Data/Resource/CachingResource.swift |
Updates cached resource streaming callback to @Sendable. |
Sources/Shared/Toolkit/Data/Resource/BufferingResource.swift |
Updates streaming callback and protects buffered accumulation with Mutex. |
Sources/Shared/Toolkit/Data/Resource/BorrowedResource.swift |
Converts borrowed resource to a struct and updates streaming callback. |
Sources/Shared/Publication/Services/Positions/PositionsService.swift |
Makes positions services Sendable and wraps positions resource access in a sendable closure. |
Sources/Shared/Publication/Services/Cover/GeneratedCoverService.swift |
Wraps cover resource access in a sendable weak-self closure. |
Sources/Navigator/EPUB/EPUBNavigatorViewModel.swift |
Pre-serves fonts on the main actor and snapshots font mappings for CSS injection. |
Sources/Navigator/EPUB/CSS/HTMLFontFamilyDeclaration.swift |
Makes font declarations Sendable and exposes associated font files. |
Sources/LCP/Content Protection/LCPDecryptor.swift |
Replaces FullLCPResource subclassing with TransformingResource composition and eagerly creates CBC plaintext size task. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@mickael-menu I'd prefer not to take this PR too far. Making |
|
@stevenzeck Agreed, if it's not worse than before and only strict concurrency warnings. |
mickael-menu
left a comment
There was a problem hiding this comment.
Thank you for making the changes @stevenzeck!
StreamableandResourceprotocols toSendableacross the codebase.streamcallback signatures to require@escaping @Sendable (Data) -> Void.TransformingResourcefrom subclass inheritance to closure-based composition.BufferingResource,TailCachingResource, andStreamablelocal accumulators to useMutexwrappers to prevent data races.PositionsServicetoSendable & AnyObjectto allow safe[weak self]closure capture.LCPDecryptorandEPUBNavigatorViewModelby pre-serving fonts on the main actor and using read-only snapshots in background mapping tasks.CBCLCPResource, the size calculation forplainTextSizeTaskhas been shifted from alazy varto a constantTaskinitialized ininitto conform toSendable.WebViewServeris main-actor isolated, font URLs are now pre-served on the@MainActorbefore mapping. A warning is logged if an unserved font is encountered during mapping.Public API Breaking Changes
Streamable,Resource,HTMLFontFamilyDeclaration, andPositionsServiceall conform to Sendable.consumecallback added@Sendable.Resource.mapandResource.mapAsStringtransform closures both added@SendableTransformingResourcechanged from open class to public final.fontFilesis a required property inHTMLFontFamilyDeclaration.PositionsService, default implementations forlinksandgetare now restricted towhere Self: AnyObjectto support safe weak capture. Any custom struct-based conformances must implement these properties manually or be changed to classes/actors.Note: This will have warnings until #769 and/or #772 are merged.
Relates to #758.