Added Box support #34
Conversation
WalkthroughThis update introduces significant enhancements to the Cryptomator iOS app for Box cloud integration. New authentication functionalities have been added, including OAuth login and token storage mechanisms. Additionally, comprehensive cloud storage operations using promises and detailed automated integration testing guidelines for developers have been incorporated. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant App as Cryptomator App
participant Box as Box API
participant TokenStorage as Token Storage
User->>App: Initiate Box Authentication
App->>Box: OAuth Request
Box-->>User: OAuth Login Page
User->>Box: Provide Credentials
Box-->>App: Access Token
App->>TokenStorage: Store Access Token
TokenStorage-->>App: Confirmation
App-->>User: Authentication Success
Poem
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: 3
Actionable comments outside the diff hunks (1)
create-integration-test-secrets-file.sh (1)
Line range hint
3-3: Replacesourcewith.to improve the portability of the script and ensure compatibility with POSIX sh.- source ./.integration-test-secrets.sh + . ./.integration-test-secrets.sh
Tests/CryptomatorCloudAccessIntegrationTests/Box/BoxCloudProviderIntegrationTests.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
You've probably seen that the CI failed: https://github.com/cryptomator/cloud-access-swift/actions/runs/8694502837/job/23843551721
Make sure to fix the formatting issues raised by SwiftFormat and SwiftLint. In the root of the project, I believe you can run:
swiftformat .
swiftlint --fixIf you want, you can also add a pre-commit hook so that you will notice these issues earlier: https://github.com/cryptomator/cloud-access-swift?tab=readme-ov-file#contributing
After that, I'll go through the PR more thoroughly. 😄
# Conflicts: # CryptomatorCloudAccess.xcodeproj/project.pbxproj
# Conflicts: # create-integration-test-secrets-file.sh
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (1)
create-integration-test-secrets-file.sh (1)
Line range hint
3-3: Replace 'source' with '.' for better POSIX compliance.- source ./.integration-test-secrets.sh + . ./.integration-test-secrets.sh
There was a problem hiding this comment.
All in all, I'm quite happy with the implementation. Just did some minor changes between 79bb7cd...eb9a34d.
This is almost ready for a merge, but I'd like to request updated READMEs (both in the root directory and in the integration tests).
We should make sure that the integration tests work reliably with the current BoxCredentialMock implementation, which requires an access token and refresh token. I think you mentioned that there are some issues with that? As a fallback, we can require the usage of a developer token, but I'm not that happy with the 60 minutes limitation, to be honest.
Edit: You can try to set expiresIn to 59 or something lower (I think 60 is the tokenRefreshThreshold) instead of 3600, so that the SDK instantly tries to refresh the token. In that case, I would even argue that we don't need to specify an access token, because you'll always get a new access token, similar to the integration tests of Google Drive and OneDrive.
README.md
Outdated
| let tokenStore = BoxTokenStore() | ||
| let viewController = ... // the presenting `UIViewController` | ||
| BoxAuthenticator.authenticate(credential: credential, from: viewController).then { |
There was a problem hiding this comment.
Creating the BoxCredential is missing here. 😉
There was a problem hiding this comment.
Fixed in commit 995defc.
| @@ -33,6 +35,21 @@ If you are building via a CI system, set these secret environment variables acco | |||
|
|
|||
| ### How to Get the Secrets | |||
|
|
|||
| #### Box | |||
|
|
|||
| To get the access token for Box, generate a developer token in the Box Developer Portal. For more detailed instructions, check out the [OAuth 2.0 Documentation from Box](https://developer.box.com/guides/authentication/oauth2/). | |||
There was a problem hiding this comment.
Just wanted to mention here that the developer token shouldn't be mixed up with the access token from OAuth. The access token can actually be extracted at the same time when extracting the refresh token, which you've described below.
However, we should experiment with a solution that doesn't require an access token at all, as I've described in a different comment. My guess is that a refresh token would be enough for Box to get a new access token.
There was a problem hiding this comment.
Fixed in commit 995defc.
…egrate developer token.
There was a problem hiding this comment.
Actionable comments posted: 2
Out of diff range and nitpick comments (1)
create-integration-test-secrets-file.sh (1)
Line range hint
3-3: Replace 'source' with '.' for POSIX compliance.- source ./.integration-test-secrets.sh + . ./.integration-test-secrets.sh
|
|
||
| private static let credential = BoxCredentialMock() | ||
| // swiftlint:disable:next force_try | ||
| private static let cloudProvider = try! BoxCloudProvider(credential: credential) |
There was a problem hiding this comment.
Avoid using 'force_try' in tests to prevent crashes during test execution.
- private static let cloudProvider = try! BoxCloudProvider(credential: credential)
+ private static let cloudProvider = try? BoxCloudProvider(credential: credential)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| private static let cloudProvider = try! BoxCloudProvider(credential: credential) | |
| private static let cloudProvider = try? BoxCloudProvider(credential: credential) |
There was a problem hiding this comment.
Thank you, but not necessary.
There was a problem hiding this comment.
@iammajid, understood! If you need further assistance or have any other requests in the future, feel free to let me know. Happy coding! 🐰
| To obtain the refresh token from Box, it is recommended to extract it from `authenticate` after a successful login. The easiest way to do this is to set a breakpoint inside the `BoxAuthenticator`: | ||
|
|
||
| ```swift | ||
| public static func authenticate(from viewController: UIViewController, tokenStore: TokenStore) -> Promise<(BoxClient, String)> { | ||
| return Promise { | ||
| // ... | ||
| fulfill((client, user.id)) // set breakpoint here | ||
| // ... | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Add detailed comments within the authenticate function to guide users on using breakpoints effectively for extracting the refresh token.
+ // Note: The breakpoint will allow you to inspect the authentication process and extract the refresh token.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| To obtain the refresh token from Box, it is recommended to extract it from `authenticate` after a successful login. The easiest way to do this is to set a breakpoint inside the `BoxAuthenticator`: | |
| ```swift | |
| public static func authenticate(from viewController: UIViewController, tokenStore: TokenStore) -> Promise<(BoxClient, String)> { | |
| return Promise { | |
| // ... | |
| fulfill((client, user.id)) // set breakpoint here | |
| // ... | |
| } | |
| ``` | |
| public static func authenticate(from viewController: UIViewController, tokenStore: TokenStore) -> Promise<(BoxClient, String)> { | |
| return Promise { | |
| // ... | |
| fulfill((client, user.id)) // set breakpoint here | |
| // Note: The breakpoint will allow you to inspect the authentication process and extract the refresh token. | |
| // ... | |
| } |
There was a problem hiding this comment.
Thank you, but not necessary.
There was a problem hiding this comment.
@iammajid, understood! If there's anything else you need, feel free to let me know. Happy coding!
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (4)
Tests/CryptomatorCloudAccessIntegrationTests/README.md (4)
39-39: Clarify the expiration details of the Box developer token to prevent confusion during testing.
Line range hint
47-47: Add a comment in theGoogleDriveAuthenticatorcode snippet to explain the purpose of the breakpoint for new developers.+ // Set a breakpoint here to extract the refresh token after successful authentication
Line range hint
91-91: In thePCloudAuthenticatorcode snippet, clarify what the developer should look for when the breakpoint is hit to make the token extraction process clearer.+ // Set a breakpoint here to capture the PCloudCredential details after successful authorization
Line range hint
170-170: The file should end with a single newline character to adhere to POSIX standards.+
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
CryptomatorCloudAccess.xcodeproj/project.pbxproj (1)
189-200: Review overall project settings for potential optimization.Consider reviewing the overall project settings to optimize build times and resource usage, especially with the addition of new components like Box.
Also applies to: 421-421
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- CryptomatorCloudAccess.xcodeproj/project.pbxproj (18 hunks)
- CryptomatorCloudAccess.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (5 hunks)
- CryptomatorCloudAccess.xcodeproj/xcshareddata/xcschemes/BoxIntegrationTests.xcscheme (1 hunks)
- Package.resolved (1 hunks)
- Package.swift (3 hunks)
- Sources/CryptomatorCloudAccess/Box/BoxAuthenticator.swift (1 hunks)
- Sources/CryptomatorCloudAccess/Box/BoxCloudProvider.swift (1 hunks)
- Sources/CryptomatorCloudAccess/Box/BoxCredential.swift (1 hunks)
- Sources/CryptomatorCloudAccess/Box/BoxIdentifierCache.swift (1 hunks)
- Sources/CryptomatorCloudAccess/Box/BoxItem.swift (1 hunks)
- Tests/CryptomatorCloudAccessIntegrationTests/Box/BoxCredentialMock.swift (1 hunks)
Files skipped from review due to trivial changes (2)
- CryptomatorCloudAccess.xcodeproj/xcshareddata/xcschemes/BoxIntegrationTests.xcscheme
- Package.resolved
Additional Context Used
Learnings (1)
Sources/CryptomatorCloudAccess/Box/BoxAuthenticator.swift (1)
User: tobihagemann URL: https://github.com/cryptomator/cloud-access-swift/pull/34 Timestamp: 2024-04-16T06:03:14.688Z Learning: The `BoxAuthenticatorError` enum in `BoxAuthenticator.swift` includes a case `invalidContext` for handling scenarios where a `UIViewController` cannot be cast to `ASWebAuthenticationPresentationContextProviding`.
Additional comments not posted (22)
Sources/CryptomatorCloudAccess/Box/BoxIdentifierCache.swift (3)
15-25: The constructor correctly sets up the in-memory database and handles its initialization errors by propagating them. Good use of resource initialization.
33-37: TheaddOrUpdate(_:)method correctly handles database operations with appropriate error propagation.
39-43: Good use of parameterized queries ininvalidate(_:)to prevent SQL injection.Sources/CryptomatorCloudAccess/Box/BoxAuthenticator.swift (1)
22-46: Theauthenticate(from:tokenStorage:)method is well-implemented with safe casting and proper error handling. It also makes good use of modern Swift concurrency features.Sources/CryptomatorCloudAccess/Box/BoxCredential.swift (3)
22-26: The constructorinit(tokenStore:)is correctly implemented with proper initialization of OAuth configuration and client.
28-42: Thedeauthenticate()method correctly handles token revocation with appropriate error handling.
44-61: ThegetUsername()method is well-implemented, handling both the presence and absence of a username correctly, and providing clear error handling.Tests/CryptomatorCloudAccessIntegrationTests/Box/BoxCredentialMock.swift (2)
35-47: The constructor ofBoxCredentialMockis well-implemented, setting up the necessary test environment and initializing the superclass correctly.
49-54: Thedeauthenticate()method inBoxCredentialMockis correctly implemented for testing purposes, setting the client to an invalid token.Sources/CryptomatorCloudAccess/Box/BoxItem.swift (3)
13-22: TheBoxItemstruct is well-defined, with clear properties and appropriate protocol conformances for database operations.
24-50: The extension ofBoxItemis well-implemented, providing initializers for different item types and correctly handling unsupported types by throwing an error.
52-58: The encoding functionality in theBoxItemextension is correctly implemented, ensuring that item properties are properly encoded for database operations.Package.swift (1)
Line range hint
17-46: The package configuration inPackage.swiftis correctly set up, including the necessary dependencies and targets for the integration of the Box SDK.CryptomatorCloudAccess.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (1)
31-39: Added dependency onbox-swift-sdk-genversion0.2.0.This addition aligns with the PR's objective to integrate Box Cloud support. Ensure that this version of the SDK is compatible with the rest of the project dependencies and the Swift version used.
Sources/CryptomatorCloudAccess/Box/BoxCloudProvider.swift (6)
18-22: Ensure proper initialization ofBoxCloudProvider.The constructor correctly initializes the
credential,identifierCache, andmaxPageSizeproperties. The use ofmax(1, min(maxPageSize, 1000))ensures thatmaxPageSizeremains within a sensible range, which is good for controlling the size of data pages handled by the provider.
24-28: Review the implementation of metadata fetching.The method
fetchItemMetadatauses a promise chain to resolve the item path and then fetch its metadata. This approach is clean and maintains readability while handling asynchronous operations effectively.
36-44: Check preconditions and handle file download correctly.The method
downloadFilecorrectly checks if the URL is a file URL and if the file already exists, returning an error if so. This prevents overwriting existing files unintentionally. The rest of the method handles the download task asynchronously, which is appropriate for network operations.
46-70: Ensure robust file upload handling.The
uploadFilemethod includes comprehensive checks for the file's existence and type before proceeding. The conditional logic aroundreplaceExistingis correctly implemented to throw an error if an attempt is made to replace a folder with a file. The use of promises to chain these checks and the final upload task is well done.
72-82: Validate folder creation logic.The method
createFolderproperly checks if the folder already exists before attempting to create it, which avoids unnecessary API calls and potential errors. The promise chain is used effectively to handle asynchronous operations.
339-373: Review deletion logic and error handling indeleteFile.The
deleteFilemethod includes comprehensive error handling, including specific checks for expired tokens and file not found errors. The use of detailed logging helps in debugging and maintaining the system. The method also updates the cache upon successful deletion, which is crucial for consistency.CryptomatorCloudAccess.xcodeproj/project.pbxproj (2)
189-200: Integration of Box-related files and framework appears correctly implemented.Ensure that all newly added files are correctly referenced and that the
BoxSdkGenframework is properly configured in the build settings.Also applies to: 421-421
189-200: Configuration settings for Box integration are correctly added.Verify that the build configurations for the new Box-related files are correctly set to ensure smooth builds and integration tests.
| func get(_ cloudPath: CloudPath) -> BoxItem? { | ||
| try? inMemoryDB.read { db in | ||
| return try BoxItem.fetchOne(db, key: cloudPath) | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider adding error logging in the get(_:) method to aid in debugging when database read operations fail.
There was a problem hiding this comment.
Thank you, but not necessary.
There was a problem hiding this comment.
@iammajid, understood! If you need any further assistance or a fresh set of eyes on other parts of your code in the future, feel free to reach out. Happy coding! 🚀
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- Sources/CryptomatorCloudAccess/Box/BoxAuthenticator.swift (1 hunks)
- Sources/CryptomatorCloudAccess/Box/BoxCredential.swift (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- Sources/CryptomatorCloudAccess/Box/BoxAuthenticator.swift
- Sources/CryptomatorCloudAccess/Box/BoxCredential.swift
| "identity" : "box-swift-sdk-gen", | ||
| "kind" : "remoteSourceControl", | ||
| "location" : "https://github.com/box/box-ios-sdk.git", | ||
| "location" : "https://github.com/box/box-swift-sdk-gen.git", | ||
| "state" : { | ||
| "revision" : "daffd86b861a5f5882655bf7a01b891f6b808c1f", | ||
| "version" : "5.5.0" | ||
| "revision" : "e5069af728c4b4f048078dfb5aed683c926da857", | ||
| "version" : "0.2.0" |
There was a problem hiding this comment.
This selection is fine. However, you've introduced further changes in this file that are unrelated. I know that Xcode sometimes does this automatically, but please revert them to avoid unwanted changes.
There was a problem hiding this comment.
Reverted in commit 0af03cd.
| value = "" | ||
| isEnabled = "YES"> | ||
| </EnvironmentVariable> | ||
| </EnvironmentVariables> |
There was a problem hiding this comment.
I would revert these changes. They don't seem to change anything useful.
There was a problem hiding this comment.
Reverted in commit 0af03cd.
| } | ||
| public var client: BoxClient | ||
|
|
||
| public init(tokenStore: TokenStorage) { |
There was a problem hiding this comment.
Rename tokenStore to tokenStorage to be more consistent.
There was a problem hiding this comment.
Fixed in commit d544b20.
| } | ||
| // Run the login flow and store the access token using tokenStorage | ||
| try await oauth.runLoginFlow(options: .init(), context: context) | ||
| // TODO: Catch error when login failed |
There was a problem hiding this comment.
Just wanted to clarify here that I meant "when login is being cancelled". I just assumed that runLoginFlow would throw a special error in that case.
There was a problem hiding this comment.
Implemented in commit d544b20.
| #if canImport(CryptomatorCloudAccessCore) | ||
| @testable import CryptomatorCloudAccessCore | ||
| #else | ||
| @testable import CryptomatorCloudAccess | ||
| #endif | ||
|
|
||
| // Custom in-memory token storage | ||
| class InMemoryTokenStore: TokenStorage { |
There was a problem hiding this comment.
Is this really needed? Can't this be used? https://github.com/box/box-swift-sdk-gen/blob/62670ce06100cbabae3482b3a89c2365b6322d57/Sources/Box/InMemoryTokenStorage.swift
There was a problem hiding this comment.
Removed in commit d544b20.
| let invalidTokenAuth = BoxDeveloperTokenAuth(token: "invalid") | ||
| client = BoxClient(auth: invalidTokenAuth) |
There was a problem hiding this comment.
I wonder if we should use a different Authentication implemention of BoxSdk, because we don't use the developer token in the main app, but an OAuth token.
If you compare BoxDeveloperTokenAuth.swift with BoxOAuth.swift, you can see the different BoxSDKErrors that can be thrown.
I'm actually a little annoyed that you can't just catch the status code 401 for this.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- CryptomatorCloudAccess.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (2 hunks)
- CryptomatorCloudAccess.xcodeproj/xcshareddata/xcschemes/BoxIntegrationTests.xcscheme (1 hunks)
Files not reviewed due to errors (2)
- CryptomatorCloudAccess.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (no review received)
- CryptomatorCloudAccess.xcodeproj/xcshareddata/xcschemes/BoxIntegrationTests.xcscheme (no review received)
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- Sources/CryptomatorCloudAccess/Box/BoxAuthenticator.swift (1 hunks)
- Sources/CryptomatorCloudAccess/Box/BoxCloudProvider.swift (1 hunks)
- Sources/CryptomatorCloudAccess/Box/BoxCredential.swift (1 hunks)
- Tests/CryptomatorCloudAccessIntegrationTests/Box/BoxCredentialMock.swift (1 hunks)
Files not reviewed due to errors (1)
- Tests/CryptomatorCloudAccessIntegrationTests/Box/BoxCredentialMock.swift (no review received)
Additional context used
Learnings (2)
Tests/CryptomatorCloudAccessIntegrationTests/Box/BoxCredentialMock.swift (1)
User: tobihagemann URL: https://github.com/cryptomator/cloud-access-swift/pull/34 Timestamp: 2024-04-16T06:03:14.688Z Learning: The `BoxAuthenticatorError` enum in `BoxAuthenticator.swift` includes a case `invalidContext` for handling scenarios where a `UIViewController` cannot be cast to `ASWebAuthenticationPresentationContextProviding`.Sources/CryptomatorCloudAccess/Box/BoxAuthenticator.swift (1)
User: tobihagemann URL: https://github.com/cryptomator/cloud-access-swift/pull/34 Timestamp: 2024-04-16T06:03:14.688Z Learning: The `BoxAuthenticatorError` enum in `BoxAuthenticator.swift` includes a case `invalidContext` for handling scenarios where a `UIViewController` cannot be cast to `ASWebAuthenticationPresentationContextProviding`.
SwiftLint
Sources/CryptomatorCloudAccess/Box/BoxCloudProvider.swift
[Warning] 320-320: Function body should span 50 lines or less excluding comments and whitespace: currently spans 51 lines (function_body_length)
[Warning] 382-382: Function body should span 50 lines or less excluding comments and whitespace: currently spans 59 lines (function_body_length)
[Warning] 382-382: Function should have 5 parameters or less: it currently has 7 (function_parameter_count)
Additional comments not posted (1)
Sources/CryptomatorCloudAccess/Box/BoxAuthenticator.swift (1)
23-54: Review ofBoxAuthenticator.authenticatemethodThis method is well-structured and uses modern Swift concurrency features. It correctly handles different error scenarios, including invalid context and login cancellation. The use of
ASWebAuthenticationSessionErrorfor specific error handling is a good practice. However, ensure that all possible errors from theASWebAuthenticationSessionare handled appropriately.
| public class BoxCredential { | ||
| public var client: BoxClient | ||
|
|
||
| public init(tokenStorage: TokenStorage) { | ||
| let config = OAuthConfig(clientId: BoxSetup.constants.clientId, clientSecret: BoxSetup.constants.clientSecret, tokenStorage: tokenStorage) | ||
| let oauth = BoxOAuth(config: config) | ||
| self.client = BoxClient(auth: oauth) | ||
| } | ||
|
|
||
| public func deauthenticate() -> Promise<Void> { | ||
| let pendingPromise = Promise<Void>.pending() | ||
|
|
||
| _Concurrency.Task { | ||
| do { | ||
| let networkSession = NetworkSession() | ||
| try await self.client.auth.revokeToken(networkSession: networkSession) | ||
| pendingPromise.fulfill(()) | ||
| } catch { | ||
| pendingPromise.reject(error) | ||
| } | ||
| } | ||
|
|
||
| return pendingPromise | ||
| } | ||
|
|
||
| public func getUsername() -> Promise<String> { | ||
| let pendingPromise = Promise<String>.pending() | ||
|
|
||
| _Concurrency.Task { | ||
| do { | ||
| let user = try await client.users.getUserMe() | ||
| if let name = user.name { | ||
| pendingPromise.fulfill(name) | ||
| } else { | ||
| pendingPromise.reject(BoxCredentialErrors.noUsername) | ||
| } | ||
| } catch { | ||
| pendingPromise.reject(error) | ||
| } | ||
| } | ||
|
|
||
| return pendingPromise | ||
| } | ||
|
|
||
| public func getUserId() -> Promise<String> { | ||
| let pendingPromise = Promise<String>.pending() | ||
|
|
||
| _Concurrency.Task { | ||
| do { | ||
| let user = try await client.users.getUserMe() | ||
| pendingPromise.fulfill(user.id) | ||
| } catch { | ||
| pendingPromise.reject(error) | ||
| } | ||
| } | ||
|
|
||
| return pendingPromise | ||
| } |
There was a problem hiding this comment.
Comprehensive review of BoxCredential methods
The implementation of deauthenticate, getUsername, and getUserId methods in BoxCredential uses Promises effectively. Each method is implemented with proper error handling and concurrency. However, consider adding more detailed logging in error handling blocks to aid in debugging and maintenance.
- pendingPromise.reject(error)
+ CloudAccessDDLogDebug("Error occurred: \(error.localizedDescription)")
+ pendingPromise.reject(error)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public class BoxCredential { | |
| public var client: BoxClient | |
| public init(tokenStorage: TokenStorage) { | |
| let config = OAuthConfig(clientId: BoxSetup.constants.clientId, clientSecret: BoxSetup.constants.clientSecret, tokenStorage: tokenStorage) | |
| let oauth = BoxOAuth(config: config) | |
| self.client = BoxClient(auth: oauth) | |
| } | |
| public func deauthenticate() -> Promise<Void> { | |
| let pendingPromise = Promise<Void>.pending() | |
| _Concurrency.Task { | |
| do { | |
| let networkSession = NetworkSession() | |
| try await self.client.auth.revokeToken(networkSession: networkSession) | |
| pendingPromise.fulfill(()) | |
| } catch { | |
| pendingPromise.reject(error) | |
| } | |
| } | |
| return pendingPromise | |
| } | |
| public func getUsername() -> Promise<String> { | |
| let pendingPromise = Promise<String>.pending() | |
| _Concurrency.Task { | |
| do { | |
| let user = try await client.users.getUserMe() | |
| if let name = user.name { | |
| pendingPromise.fulfill(name) | |
| } else { | |
| pendingPromise.reject(BoxCredentialErrors.noUsername) | |
| } | |
| } catch { | |
| pendingPromise.reject(error) | |
| } | |
| } | |
| return pendingPromise | |
| } | |
| public func getUserId() -> Promise<String> { | |
| let pendingPromise = Promise<String>.pending() | |
| _Concurrency.Task { | |
| do { | |
| let user = try await client.users.getUserMe() | |
| pendingPromise.fulfill(user.id) | |
| } catch { | |
| pendingPromise.reject(error) | |
| } | |
| } | |
| return pendingPromise | |
| } | |
| public class BoxCredential { | |
| public var client: BoxClient | |
| public init(tokenStorage: TokenStorage) { | |
| let config = OAuthConfig(clientId: BoxSetup.constants.clientId, clientSecret: BoxSetup.constants.clientSecret, tokenStorage: tokenStorage) | |
| let oauth = BoxOAuth(config: config) | |
| self.client = BoxClient(auth: oauth) | |
| } | |
| public func deauthenticate() -> Promise<Void> { | |
| let pendingPromise = Promise<Void>.pending() | |
| _Concurrency.Task { | |
| do { | |
| let networkSession = NetworkSession() | |
| try await self.client.auth.revokeToken(networkSession: networkSession) | |
| pendingPromise.fulfill(()) | |
| } catch { | |
| CloudAccessDDLogDebug("Error occurred: \(error.localizedDescription)") | |
| pendingPromise.reject(error) | |
| } | |
| } | |
| return pendingPromise | |
| } | |
| public func getUsername() -> Promise<String> { | |
| let pendingPromise = Promise<String>.pending() | |
| _Concurrency.Task { | |
| do { | |
| let user = try await client.users.getUserMe() | |
| if let name = user.name { | |
| pendingPromise.fulfill(name) | |
| } else { | |
| pendingPromise.reject(BoxCredentialErrors.noUsername) | |
| } | |
| } catch { | |
| CloudAccessDDLogDebug("Error occurred: \(error.localizedDescription)") | |
| pendingPromise.reject(error) | |
| } | |
| } | |
| return pendingPromise | |
| } | |
| public func getUserId() -> Promise<String> { | |
| let pendingPromise = Promise<String>.pending() | |
| _Concurrency.Task { | |
| do { | |
| let user = try await client.users.getUserMe() | |
| pendingPromise.fulfill(user.id) | |
| } catch { | |
| CloudAccessDDLogDebug("Error occurred: \(error.localizedDescription)") | |
| pendingPromise.reject(error) | |
| } | |
| } | |
| return pendingPromise | |
| } |
| return pendingPromise | ||
| } | ||
|
|
||
| private func normalFileUpload(for parentItem: BoxItem, from localURL: URL, to cloudPath: CloudPath, onTaskCreation: ((URLSessionUploadTask?) -> Void)?) -> Promise<CloudItemMetadata> { |
There was a problem hiding this comment.
Function length and parameter count issues in BoxCloudProvider
The normalFileUpload and chunkedFileUpload functions exceed the recommended line count and parameter count. Consider refactoring these functions to improve readability and maintainability.
- private func chunkedFileUpload(client: BoxClient, from localURL: URL, fileSize: Int64, targetFileName: String, parentItem: BoxItem, cloudPath: CloudPath, onTaskCreation: ((URLSessionUploadTask?) -> Void)?)
+ private func chunkedFileUpload(uploadSession: UploadSessionDetails)Refactor to use a struct for passing multiple parameters.
Also applies to: 382-382
Tools
SwiftLint
[Warning] 320-320: Function body should span 50 lines or less excluding comments and whitespace: currently spans 51 lines (function_body_length)
There was a problem hiding this comment.
That's okay for now.
There was a problem hiding this comment.
@iammajid, understood! If you decide to revisit this in the future or need any assistance, feel free to reach out. Happy coding! 🚀
Learnings added
User: iammajid
PR: cryptomator/cloud-access-swift#34
File: Sources/CryptomatorCloudAccess/Box/BoxCloudProvider.swift:320-320
Timestamp: 2024-06-21T13:51:55.137Z
Learning: iammajid prefers to keep the function lengths and parameter counts as they are in the `BoxCloudProvider` for now.
Note: Learnings are effective only in the context of specific code segments. To apply general coding guidelines, please configure review instructions.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- Tests/CryptomatorCloudAccessIntegrationTests/README.md (2 hunks)
Additional context used
Learnings (1)
Common learnings
User: iammajid PR: cryptomator/cloud-access-swift#34 File: Sources/CryptomatorCloudAccess/Box/BoxCloudProvider.swift:320-320 Timestamp: 2024-06-21T13:51:55.259Z Learning: iammajid prefers to keep the function lengths and parameter counts as they are in the `BoxCloudProvider` for now.
LanguageTool
Tests/CryptomatorCloudAccessIntegrationTests/README.md
[style] ~32-~32: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...on, you have to run that script again. If you are building via a CI system, set t...
Markdownlint
Tests/CryptomatorCloudAccessIntegrationTests/README.md
172-172: null (MD047, single-trailing-newline)
Files should end with a single newline character
Additional comments not posted (3)
Tests/CryptomatorCloudAccessIntegrationTests/README.md (3)
39-41: Review the new instructions for Box developer tokens.The instructions are clear and emphasize the practicality of using developer tokens for integration tests. Please verify that the external link to the Box OAuth 2.0 Documentation is correct and accessible to ensure users can follow these steps without issues.
Line range hint
32-32: Consider varying sentence structure.To enhance readability, consider rewording the sentences to avoid repetitive beginnings. This is a minor style improvement that could make the instructions clearer.
Line range hint
172-172: Add a trailing newline at the end of the file.To comply with Markdown formatting standards, please ensure the file ends with a single newline character.
Introduces Box Cloud as a new cloud provider in cloud-access-swift
This PR integrates Box Cloud into the cloud-access-swift framework. It includes the implementation of the CloudProvider functionalities, OAuth2 authentication, addition of necessary credentials, and the creation of integration tests.
All integration tests are running successfully without any errors.