Fix background URLSession identifier collisions #351
Conversation
WalkthroughThe update focuses on enhancing Cryptomator's cloud storage interaction, specifically improving background session support for various cloud providers. Changes include updating dependencies, refining provider management, and addressing specific issues like the "Content Unavailable" error when accessing multiple vaults on OneDrive. These modifications aim to streamline cloud service setups and improve session handling for a smoother user experience. Changes
Assessment against linked issues
Possibly related issues
Poem
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (6)
- Cryptomator.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (1 hunks)
- Cryptomator/AppDelegate.swift (1 hunks)
- CryptomatorCommon/Package.swift (1 hunks)
- CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/CloudProviderDBManager.swift (2 hunks)
- CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/VaultDBManager.swift (1 hunks)
- FileProviderExtensionUI/RootViewController.swift (1 hunks)
Additional comments not posted (8)
CryptomatorCommon/Package.swift (1)
29-29: Ensure the branchfeature/unique-identifier-bg-sessionofcloud-access-swiftcontains all necessary changes and is stable for production use. It's crucial to verify that this branch is up-to-date with any critical updates from the main or release branches to avoid missing important fixes or features.Cryptomator/AppDelegate.swift (1)
Line range hint
1-1: Ensure that the removal ofCloudProviderDBManager.shared.useBackgroundSessionassignment inAppDelegatealigns with the new approach for managing background sessions at a more granular level. Verify that all necessary adjustments have been made elsewhere in the code to support this change effectively.Cryptomator.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (1)
62-63: The update to thecloud-access-swiftpackage to use the branchfeature/unique-identifier-bg-sessionis noted. Ensure that this branch is stable and contains all necessary features and fixes for the application. It's also important to plan for the transition back to a versioned release once the required features are merged into a stable release ofcloud-access-swift.CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/CloudProviderDBManager.swift (3)
22-27: The introduction of theCachedProviderstruct is a good approach for managing cloud providers with background session support. This encapsulation allows for cleaner management of session-related information. Ensure that the usage ofCachedProviderthroughout theCloudProviderDBManageraligns with the intended design and that thread safety is considered if access from multiple threads is expected.
49-53: When retrieving a background session provider, it's crucial to handle the case where a provider with the specifiedsessionIdentifierdoes not exist. Consider adding error handling or logging to inform about the fallback to creating a new provider, ensuring that this behavior is intentional and understood.
109-163: The methodcreateBackgroundSessionProvidereffectively creates providers with background session support. However, ensure that themaxPageSizeForFileProviderlimit is well-documented and justified, as it impacts the number of items fetched in a single call. Additionally, verify that all providers support the creation with background sessions and that the session identifiers are managed securely and efficiently.FileProviderExtensionUI/RootViewController.swift (1)
Line range hint
1-1: Ensure that the removal ofCloudProviderDBManager.shared.useBackgroundSessionassignment inRootViewControlleraligns with the new approach for managing background sessions at a more granular level. Verify that all necessary adjustments have been made elsewhere in the code to support this change effectively.CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/VaultDBManager.swift (1)
486-487: Using thevaultUIDas the background URLSession identifier is a good approach to prevent identifier collisions.Ensure that the rest of the system correctly handles these unique session identifiers, particularly in terms of session reuse and management, to avoid potential performance issues.
This reverts commit 2d27749.
…SwiftLint is not necessary
This PR fixes #345.
It currently relies on the
cloud-access-swiftbranch and can be used right now to test the PR: cryptomator/cloud-access-swift#33Those changes are needed since issues appeared in the
FileProviderExtensionwhere we tried to use the same credential, which we usually used to make the identifier unique, for multiple vaults. Therefore, it was possible that more than one backgroundURLSessiongets created with the same identifier which then results in a failingURLSession.Previously we could workaround this issue by just cache each
CloudProviderbut it seems like iOS creates now a own process for each domain.Summary by CodeRabbit
New Features
Improvements
Bug Fixes