Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storage async await #8289

Merged
merged 18 commits into from
Jul 23, 2021
Merged

Storage async await #8289

merged 18 commits into from
Jul 23, 2021

Conversation

paulb777
Copy link
Member

@paulb777 paulb777 commented Jun 22, 2021

Storage Integration Tests are converted to async/await and four new APIs are added to cover cases that don't get an automatic Xcode conversion.

If you want to experiment, the test infra is green here.

  • Use Xcode 13
  • Update Credentials.swift
  • Add a GoogleService-Info.plist to the AppHost target
  • pod gen FirebaseStorageSwift --auto-open --local-sources=./ --platforms=ios
    Xcode 13+ automatically converts Objective C APIs that return void and take a completion block parameter to an additional async API.

Googlers can see the API proposal at go/firebase-storage-async-await1. An excerpt follows:

The automatic API conversions for Storage are shown in Column G of this spreadsheet designed by @peterfriese. Note that the APIs that return a StorageDownloadTask or StorageUploadTask are not converted.

For Storage, Xcode automatically converts the following APIs to async versions as shown here:

open func downloadURL(completion: @escaping (URL?, Error?) -> Void) open func downloadURL() async throws -> URL
open func listAll(completion: @escaping (StorageListResult, Error?) -> Void) open func listAll() async throws -> StorageListResult
open func list(maxResults: Int64, completion: @escaping (StorageListResult, Error?) -> Void) open func list(maxResults: Int64) async throws -> StorageListResult
open func list(maxResults: Int64, pageToken: String, completion: @escaping (StorageListResult, Error?) -> Void) open func list(maxResults: Int64, pageToken: String) async throws -> StorageListResult
open func getMetadata(completion: @escaping (StorageMetadata?, Error?) -> Void) open func getMetadata() async throws -> StorageMetadata
open func updateMetadata(_ metadata: StorageMetadata, completion: ((StorageMetadata?, Error?) -> Void)? = nil) open func updateMetadata(_ metadata: StorageMetadata) async throws -> StorageMetadata
open func delete(completion: ((Error?) -> Void)? = nil) open func delete() async throws

The following APIs have completion parameters but do not get an automatic async API generated because they do not have a Void return value:

Screen Shot 2021-07-12 at 9 38 52 AM

I implemented the following four APIs (full implementation here) to fill in the gap by providing APIs to do the implementation without the task handle return value:

Screen Shot 2021-07-13 at 9 19 23 AM

The functions are renamed to avoid collisions with the existing API.

@google-oss-bot
Copy link

google-oss-bot commented Jun 22, 2021

Coverage Report

Affected SDKs

No changes between base commit (d550a72) and head commit (2bffe65).

Test Logs

@paulb777 paulb777 force-pushed the pb-storage-async branch 2 times, most recently from 4007610 to ffdbc7b Compare July 1, 2021 14:31
Copy link
Member

@ncooke3 ncooke3 left a comment

Choose a reason for hiding this comment

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

This is looking really neat! 😎

@paulb777 paulb777 marked this pull request as ready for review July 12, 2021 16:45
@paulb777 paulb777 changed the title Storage async await WIP Storage async await Jul 12, 2021
@paulb777
Copy link
Member Author

API change is now approved. The PR is ready for final review and merge.

@paulb777 paulb777 assigned ncooke3 and unassigned ryanwilson Jul 22, 2021
FirebaseStorageSwift/Sources/AsyncAwait.swift Outdated Show resolved Hide resolved
FirebaseStorageSwift/Sources/AsyncAwait.swift Outdated Show resolved Hide resolved
FirebaseStorageSwift/Sources/AsyncAwait.swift Outdated Show resolved Hide resolved
FirebaseStorageSwift/CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member Author

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

Thanks for the thorough review @ncooke3 . Lots of good catches! 💯

Copy link
Member

@ncooke3 ncooke3 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@paulb777 paulb777 merged commit 2d4e139 into master Jul 23, 2021
@paulb777 paulb777 deleted the pb-storage-async branch July 23, 2021 22:45
@firebase firebase locked and limited conversation to collaborators Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants