Skip to content

Make Streamable and Resource protocols Sendable #801

Merged
mickael-menu merged 23 commits into
readium:swift6from
stevenzeck:refactor/resource
Jun 2, 2026
Merged

Make Streamable and Resource protocols Sendable #801
mickael-menu merged 23 commits into
readium:swift6from
stevenzeck:refactor/resource

Conversation

@stevenzeck
Copy link
Copy Markdown
Contributor

@stevenzeck stevenzeck commented May 27, 2026

  1. Conforms Streamable and Resource protocols to Sendable across the codebase.
  2. Updates the stream callback signatures to require @escaping @Sendable (Data) -> Void.
  3. Refactors TransformingResource from subclass inheritance to closure-based composition.
  4. Converts BufferingResource, TailCachingResource, and Streamable local accumulators to use Mutex wrappers to prevent data races.
  5. Conforms PositionsService to Sendable & AnyObject to allow safe [weak self] closure capture.
  6. Resolves strict concurrency warnings in LCPDecryptor and EPUBNavigatorViewModel by pre-serving fonts on the main actor and using read-only snapshots in background mapping tasks.
  7. In CBCLCPResource, the size calculation for plainTextSizeTask has been shifted from a lazy var to a constant Task initialized in init to conform to Sendable.
  8. Because WebViewServer is main-actor isolated, font URLs are now pre-served on the @MainActor before mapping. A warning is logged if an unserved font is encountered during mapping.

Public API Breaking Changes

  1. Streamable, Resource, HTMLFontFamilyDeclaration, and PositionsService all conform to Sendable.
  2. The consume callback added @Sendable.
  3. Resource.map and Resource.mapAsString transform closures both added @Sendable
  4. TransformingResource changed from open class to public final.
  5. fontFiles is a required property in HTMLFontFamilyDeclaration.
  6. In PositionsService, default implementations for links and get are now restricted to where Self: AnyObject to 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.

@stevenzeck stevenzeck changed the base branch from develop to swift6 May 27, 2026 01:54
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Sendable requirements to Streamable, 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.

Comment thread Sources/Shared/Toolkit/Data/Streamable.swift
Comment thread Sources/Shared/Toolkit/Data/Resource/TransformingResource.swift
Comment thread Sources/LCP/Content Protection/LCPDecryptor.swift Outdated
Comment thread Sources/LCP/Content Protection/LCPDecryptor.swift Outdated
Comment thread Sources/Navigator/EPUB/EPUBNavigatorViewModel.swift Outdated
Comment thread Sources/Navigator/EPUB/EPUBNavigatorViewModel.swift Outdated
@stevenzeck
Copy link
Copy Markdown
Contributor Author

stevenzeck commented May 30, 2026

@mickael-menu I'd prefer not to take this PR too far. Making LCPLicense conform to Sendable will lead to be more warnings, and fixing them leads to a few more. More refactoring can be done through other PRs. Thoughts?

@mickael-menu
Copy link
Copy Markdown
Member

@stevenzeck Agreed, if it's not worse than before and only strict concurrency warnings.

Comment thread Tests/SharedTests/Toolkit/HTTP/HTTPResourceTests.swift Outdated
Comment thread Sources/LCP/Content Protection/LCPDecryptor.swift
Comment thread Sources/Shared/Toolkit/Data/Resource/TransformingResource.swift Outdated
Comment thread Sources/Shared/Publication/Services/Positions/PositionsService.swift Outdated
Comment thread Sources/Shared/Publication/Services/Positions/PositionsService.swift Outdated
Comment thread Sources/Shared/Toolkit/Data/Streamable.swift
Comment thread Sources/Navigator/EPUB/EPUBNavigatorViewModel.swift Outdated
Comment thread Sources/Navigator/EPUB/CSS/HTMLFontFamilyDeclaration.swift
Copy link
Copy Markdown
Member

@mickael-menu mickael-menu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making the changes @stevenzeck!

@mickael-menu mickael-menu merged commit 578fc5e into readium:swift6 Jun 2, 2026
5 checks passed
@mickael-menu mickael-menu deleted the refactor/resource branch June 2, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants