add swift demo #458
add swift demo #458
Conversation
PR Review: Swift Demo Addition
|
Summary of ChangesHello @YangSen-qn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Swift demonstration application, Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Swift demo application for the Qiniu SDK. While it's a good starting point, I've identified several critical and high-severity issues that need attention. These include hardcoded placeholder values that will cause the application to fail (e.g., an empty upload token), incorrect configurations such as a hardcoded "zip" mime type for image uploads and an excessively high deployment target, and missing permission descriptions that will lead to runtime crashes on modern iOS versions. My review includes specific code suggestions to resolve these problems and enhance the overall robustness and usability of the demo.
| <plist version="1.0"> | ||
| <dict> | ||
| <key>NSPhotoLibraryUsageDescription</key> | ||
| <string></string> |
There was a problem hiding this comment.
The NSPhotoLibraryUsageDescription key's value is empty. Since the app needs to access the photo library to allow users to select images for upload, you must provide a user-facing string explaining why this permission is required. On recent iOS versions, failing to provide this description will cause your app to crash when the photo picker is presented.
<string>This app needs access to your photo library to select and upload images.</string>
| } | ||
|
|
||
| @IBAction func upload(_ sender: UIButton) { | ||
| let token = "" |
There was a problem hiding this comment.
The upload token is hardcoded as an empty string. This will cause all upload attempts to fail with an authentication error. For this demo to be functional, a valid upload token must be provided. It's recommended to fetch this token from a server at runtime or provide clear instructions on how to obtain and configure a valid token.
| let token = "" | |
| let token = "PASTE_YOUR_TOKEN_HERE" |
| GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; | ||
| GCC_WARN_UNUSED_FUNCTION = YES; | ||
| GCC_WARN_UNUSED_VARIABLE = YES; | ||
| IPHONEOS_DEPLOYMENT_TARGET = 18.4; |
There was a problem hiding this comment.
The IPHONEOS_DEPLOYMENT_TARGET is set to 18.4, which is likely a beta or unreleased version of iOS. This will severely limit the devices on which the demo can run. It's recommended to lower this to a more widely adopted version (e.g., 14.0) to ensure the demo is accessible to more users, unless a feature specific to iOS 18.4 is required.
IPHONEOS_DEPLOYMENT_TARGET = 14.0;
| let option = QNUploadOption.init(mime: "zip", progressHandler: { key, progress in | ||
| print("key:\(String(describing: key)) progress:\(progress)") | ||
| }, params: nil, checkCrc: false, cancellationSignal: nil); |
There was a problem hiding this comment.
The mime type is hardcoded to "zip", which is incorrect for image uploads. This can cause problems for server-side processing or when trying to view the uploaded file. You should pass nil to let the SDK attempt to infer the mime type, which is a more robust approach for handling various image formats.
| let option = QNUploadOption.init(mime: "zip", progressHandler: { key, progress in | |
| print("key:\(String(describing: key)) progress:\(progress)") | |
| }, params: nil, checkCrc: false, cancellationSignal: nil); | |
| let option = QNUploadOption.init(mime: nil, progressHandler: { key, progress in | |
| print("key:\(String(describing: key)) progress:\(progress)") | |
| }, params: nil, checkCrc: false, cancellationSignal: nil); |
| let filePath = Bundle.main.path(forResource: "image.jpg", ofType: nil) | ||
| uploader?.putFile(filePath, key: "iOS-demo-test-a", token: token, complete: { (response, key, responseData) in | ||
| if response?.isOK == true { | ||
| print("upload success:\(String(describing: responseData))") | ||
| } else { | ||
| print("upload fail:\(String(describing: response))") | ||
| } | ||
| }, option: option) |
There was a problem hiding this comment.
This block has two issues:
- Potential Crash: The path for
image.jpgis retrieved from the bundle without checking if it'snil. If the file doesn't exist in the bundle, this will likely lead to a crash whenputFileis called. You should useguard letto safely unwrap the path and handle the error case. - Hardcoded Key: The upload
keyis hardcoded, which means uploads will overwrite the same file. It's better to generate a unique key for each upload to avoid this.
guard let filePath = Bundle.main.path(forResource: "image.jpg", ofType: nil) else {
print("Error: image.jpg not found in the project bundle.")
// TODO: Show an alert to the user.
return
}
let key = "iOS-demo-test-\(UUID().uuidString)"
uploader?.putFile(filePath, key: key, token: token, complete: { (response, key, responseData) in
if response?.isOK == true {
print("upload success:\(String(describing: responseData))")
} else {
print("upload fail:\(String(describing: response))")
}
}, option: option)| ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor; | ||
| CODE_SIGN_STYLE = Automatic; | ||
| CURRENT_PROJECT_VERSION = 1; | ||
| DEVELOPMENT_TEAM = W2TP34G2MM; |
There was a problem hiding this comment.
The DEVELOPMENT_TEAM is hardcoded. This makes it difficult for other developers to build the project, as they would need to manually change this ID to their own. It is recommended to configure this setting to use automatic signing in Xcode, which avoids committing team-specific information to the repository.
| <key>NSPhotoLibraryUsageDescription</key> | ||
| <string></string> | ||
| <key>NSCameraUsageDescription</key> | ||
| <string></string> |
There was a problem hiding this comment.
The NSCameraUsageDescription key's value is empty. The ZLPhotoBrowser library may allow users to take a new photo using the camera. If this functionality is intended to be available, you should provide a user-facing string explaining why camera access is needed. If the app will not use the camera, you can remove this key.
<string>This app needs access to your camera to take and upload photos.</string>
| }, params: nil, checkCrc: false, cancellationSignal: nil); | ||
|
|
||
| if asset != nil { | ||
| uploader?.put(asset, key: "iOS-demo-test", token: token, complete: { (response, key, responseData) in |
There was a problem hiding this comment.
The upload key is hardcoded. This will cause every upload to overwrite the same file in your storage bucket. To prevent accidental data loss, it's better to generate a unique key for each upload. A common approach is to use a UUID or a timestamp.
| uploader?.put(asset, key: "iOS-demo-test", token: token, complete: { (response, key, responseData) in | |
| uploader?.put(asset, key: "iOS-demo-\(UUID().uuidString)", token: token, complete: { (response, key, responseData) in |
No description provided.