Skip to content

Commit f8acfda

Browse files
committed
Address review feedback
1 parent c47f35f commit f8acfda

File tree

7 files changed

+41
-75
lines changed

7 files changed

+41
-75
lines changed

FirebaseCombineSwift/Sources/Storage/StorageReference+Combine.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@
8282
/// The publisher will emit events on the **main** thread.
8383
///
8484
/// - Parameters:
85-
/// - size: The maximum size in bytes to download. If the download exceeds this size
85+
/// - size: The maximum size in bytes to download. If the download exceeds this size,
8686
/// the task will be cancelled and an error will be returned.
8787
///
8888
/// - Returns: A publisher emitting a `Data` instance. The publisher will emit on the *main* thread.

FirebaseStorage/Sources/Public/FirebaseStorage/FIRStorageReference.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ NS_SWIFT_NAME(putData(_:metadata:));
178178
* Asynchronously downloads the object at the FIRStorageReference to an NSData object in memory.
179179
* An NSData of the provided max size will be allocated, so ensure that the device has enough free
180180
* memory to complete the download. For downloading large files, writeToFile may be a better option.
181-
* @param size The maximum size in bytes to download. If the download exceeds this size
181+
* @param size The maximum size in bytes to download. If the download exceeds this size,
182182
* the task will be cancelled and an error will be returned.
183183
* @param completion A completion block that either returns the object data on success,
184184
* or an error on failure.
@@ -194,7 +194,7 @@ NS_SWIFT_NAME(putData(_:metadata:));
194194
/**
195195
* Asynchronously retrieves a long lived download URL with a revokable token.
196196
* This can be used to share the file with others, but can be revoked by a developer
197-
* in the Firebase Console if desired.
197+
* in the Firebase Console.
198198
* @param completion A completion block that either returns the URL on success,
199199
* or an error on failure.
200200
*/

FirebaseStorageSwift/CHANGELOG.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# 8.5.0-beta
22
- Added four APIs to augment automatically generated `async/await` APIs. See
3-
details via Xcode completion and at
4-
https://github.com/firebase/firebase-ios-sdk/blob/96d60a6d472b6fed1651d5e7a0e7495230c220ec/FirebaseStorageSwift/Sources/AsyncAwait.swift.
3+
details via Xcode completion and at the
4+
[source](https://github.com/firebase/firebase-ios-sdk/blob/96d60a6d472b6fed1651d5e7a0e7495230c220ec/FirebaseStorageSwift/Sources/AsyncAwait.swift).
55
Feedback appreciated about Firebase and `async/await`. (#8289)
66

77
# v0.1

FirebaseStorageSwift/Sources/AsyncAwait.swift

+8-7
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,12 @@ import FirebaseStorage
1818
@available(iOS 15, *)
1919
public extension StorageReference {
2020
/// Asynchronously downloads the object at the StorageReference to a Data object in memory.
21-
/// A Data object of the provided max size will be allocated, so ensure that the device has enough free
22-
/// memory to complete the download. For downloading large files, write may be a better option.
21+
/// A Data object of the provided max size will be allocated, so ensure that the device has
22+
/// enough free memory to complete the download. For downloading large files, the `write`
23+
/// API may be a better option.
2324
///
2425
/// - Parameters:
25-
/// - size: The maximum size in bytes to download. If the download exceeds this size
26+
/// - size: The maximum size in bytes to download. If the download exceeds this size,
2627
/// the task will be cancelled and an error will be thrown.
2728
/// - Returns: Data object.
2829
func data(maxSize: Int64) async throws -> Data {
@@ -36,14 +37,14 @@ import FirebaseStorage
3637
}
3738

3839
/// Asynchronously uploads data to the currently specified StorageReference.
39-
/// This is not recommended for large files, and one should instead upload a file from disk.
40-
/// in the Firebase Console if desired.
40+
/// This is not recommended for large files, and one should instead upload a file from disk
41+
/// from the Firebase Console.
4142
///
4243
/// - Parameters:
4344
/// - uploadData: The Data to upload.
4445
/// - metadata: Optional StorageMetadata containing additional information (MIME type, etc.)
4546
/// about the object being uploaded.
46-
/// - Returns: StorageMetadata containing additional information about the object being uploaded.
47+
/// - Returns: StorageMetadata with additional information about the object being uploaded.
4748
func putDataAsync(_ uploadData: Data,
4849
metadata: StorageMetadata? = nil) async throws -> StorageMetadata {
4950
typealias MetadataContinuation = CheckedContinuation<StorageMetadata, Error>
@@ -61,7 +62,7 @@ import FirebaseStorage
6162
/// - url: A URL representing the system file path of the object to be uploaded.
6263
/// - metadata: Optional StorageMetadata containing additional information (MIME type, etc.)
6364
/// about the object being uploaded.
64-
/// - Returns: StorageMetadata containing additional information about the object being uploaded.
65+
/// - Returns: StorageMetadata with additional information about the object being uploaded.
6566
func putFileAsync(from url: URL,
6667
metadata: StorageMetadata? = nil) async throws -> StorageMetadata {
6768
typealias MetadataContinuation = CheckedContinuation<StorageMetadata, Error>

FirebaseStorageSwift/Sources/Result.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ private func getResultCallback<T>(completion: @escaping (Result<T, Error>) -> Vo
4343
public extension StorageReference {
4444
/// Asynchronously retrieves a long lived download URL with a revokable token.
4545
/// This can be used to share the file with others, but can be revoked by a developer
46-
/// in the Firebase Console if desired.
46+
/// in the Firebase Console.
4747
///
4848
/// - Parameters:
4949
/// - completion: A completion block returning a `Result` enum with either a URL or an `Error`.
@@ -53,7 +53,7 @@ public extension StorageReference {
5353

5454
/// Asynchronously downloads the object at the `StorageReference` to a `Data` object.
5555
/// A `Data` of the provided max size will be allocated, so ensure that the device has enough
56-
/// memory to complete. For downloading large files, writeToFile may be a better option.
56+
/// memory to complete. For downloading large files, the `write` API may be a better option.
5757

5858
/// - Parameters:
5959
/// - maxSize: The maximum size in bytes to download.

FirebaseStorageSwift/Tests/Integration/StorageAsyncAwait.swift

+24-59
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,21 @@ import XCTest
4949
let result = try await ref.putDataAsync(data)
5050
XCTAssertNotNil(result)
5151
_ = try await ref.delete()
52+
// Next delete should fail and verify the first delete succeeded.
53+
do {
54+
_ = try await ref.delete()
55+
} catch {
56+
XCTAssertEqual((error as NSError).code, StorageErrorCode.objectNotFound.rawValue)
57+
}
5258
}
5359

54-
func testDeleteWithNilCompletion() async throws {
60+
func testDeleteAfterPut() async throws {
5561
let ref = storage.reference(withPath: "ios/public/fileToDelete")
5662
let data = try XCTUnwrap("Hello Swift World".data(using: .utf8), "Data construction failed")
5763
let result = try await ref.putDataAsync(data)
5864
XCTAssertNotNil(result)
65+
let result2: Void = try await ref.delete()
66+
XCTAssertNotNil(result2)
5967
}
6068

6169
func testSimplePutData() async throws {
@@ -67,14 +75,15 @@ import XCTest
6775

6876
func testSimplePutSpecialCharacter() async throws {
6977
let ref = storage.reference(withPath: "ios/public/-._~!$'()*,=:@&+;")
70-
let data = try XCTUnwrap("Hello Swift World".data(using: .utf8), "Data construction failed")
78+
let data = try XCTUnwrap("Hello Swift World-._~!$'()*,=:@&+;".data(using: .utf8),
79+
"Data construction failed")
7180
let result = try await ref.putDataAsync(data)
7281
XCTAssertNotNil(result)
7382
}
7483

7584
func testSimplePutDataInBackgroundQueue() async throws {
76-
actor MyBackground {
77-
func doit(_ ref: StorageReference) async throws -> StorageMetadata {
85+
actor Background {
86+
func uploadData(_ ref: StorageReference) async throws -> StorageMetadata {
7887
let data = try XCTUnwrap(
7988
"Hello Swift World".data(using: .utf8),
8089
"Data construction failed"
@@ -84,7 +93,7 @@ import XCTest
8493
}
8594
}
8695
let ref = storage.reference(withPath: "ios/public/testBytesUpload")
87-
let result = try await MyBackground().doit(ref)
96+
let result = try await Background().uploadData(ref)
8897
XCTAssertNotNil(result)
8998
}
9099

@@ -106,38 +115,8 @@ import XCTest
106115
}
107116
}
108117

109-
func testSimplePutFile() throws {
110-
let expectation = self.expectation(description: #function)
111-
let putFileExpectation = self.expectation(description: "putFile")
112-
let ref = storage.reference(withPath: "ios/public/testSimplePutFile")
113-
let data = try XCTUnwrap("Hello Swift World".data(using: .utf8), "Data construction failed")
114-
let tmpDirURL = URL(fileURLWithPath: NSTemporaryDirectory())
115-
let fileURL = tmpDirURL.appendingPathComponent("hello.txt")
116-
try data.write(to: fileURL, options: .atomicWrite)
117-
let task = ref.putFile(from: fileURL) { result in
118-
self.assertResultSuccess(result)
119-
putFileExpectation.fulfill()
120-
}
121-
122-
task.observe(StorageTaskStatus.success) { snapshot in
123-
XCTAssertEqual(snapshot.description, "<State: Success>")
124-
expectation.fulfill()
125-
}
126-
127-
var uploadedBytes: Int64 = -1
128-
129-
task.observe(StorageTaskStatus.progress) { snapshot in
130-
XCTAssertTrue(snapshot.description.starts(with: "<State: Progress") ||
131-
snapshot.description.starts(with: "<State: Resume"))
132-
guard let progress = snapshot.progress else {
133-
XCTFail("Failed to get snapshot.progress")
134-
return
135-
}
136-
XCTAssertGreaterThanOrEqual(progress.completedUnitCount, uploadedBytes)
137-
uploadedBytes = progress.completedUnitCount
138-
}
139-
waitForExpectations()
140-
}
118+
// TODO: Update this function when the task handle APIs are updated for the new Swift Concurrency.
119+
func testSimplePutFile() throws {}
141120

142121
func testAttemptToUploadDirectoryShouldFail() async throws {
143122
// This `.numbers` file is actually a directory.
@@ -200,14 +179,14 @@ import XCTest
200179
}
201180

202181
func testSimpleGetDataInBackgroundQueue() async throws {
203-
actor MyBackground {
204-
func doit(_ ref: StorageReference) async throws -> Data {
182+
actor Background {
183+
func data(from ref: StorageReference) async throws -> Data {
205184
XCTAssertFalse(Thread.isMainThread)
206185
return try await ref.data(maxSize: 1024 * 1024)
207186
}
208187
}
209188
let ref = storage.reference(withPath: "ios/public/1mb2")
210-
let result = try await MyBackground().doit(ref)
189+
let result = try await Background().data(from: ref)
211190
XCTAssertNotNil(result)
212191
}
213192

@@ -233,9 +212,8 @@ import XCTest
233212
let downloadURL = try await ref.downloadURL()
234213
let testRegex = try NSRegularExpression(pattern: downloadURLPattern)
235214
let urlString = downloadURL.absoluteString
236-
XCTAssertEqual(testRegex.numberOfMatches(in: urlString,
237-
range: NSRange(location: 0,
238-
length: urlString.count)), 1)
215+
let range = NSRange(location: 0, length: urlString.count)
216+
XCTAssertNotNil(testRegex.firstMatch(in: urlString, options: [], range: range))
239217
}
240218

241219
func testAsyncWrite() async throws {
@@ -350,10 +328,7 @@ import XCTest
350328
let listResult = try await ref.list(maxResults: 2)
351329
XCTAssertEqual(listResult.items, [ref.child("a"), ref.child("b")])
352330
XCTAssertEqual(listResult.prefixes, [])
353-
guard let pageToken = listResult.pageToken else {
354-
XCTFail("pageToken should not be nil")
355-
return
356-
}
331+
let pageToken = try XCTUnwrap(listResult.pageToken)
357332
let listResult2 = try await ref.list(maxResults: 2, pageToken: pageToken)
358333
XCTAssertEqual(listResult2.items, [])
359334
XCTAssertEqual(listResult2.prefixes, [ref.child("prefix")])
@@ -369,23 +344,13 @@ import XCTest
369344
}
370345

371346
private func waitForExpectations() {
372-
let kFIRStorageIntegrationTestTimeout = 60.0
373-
waitForExpectations(timeout: kFIRStorageIntegrationTestTimeout,
347+
let kTestTimeout = 60.0
348+
waitForExpectations(timeout: kTestTimeout,
374349
handler: { (error) -> Void in
375350
if let error = error {
376351
print(error)
377352
}
378353
})
379354
}
380-
381-
private func assertResultSuccess<T>(_ result: Result<T, Error>,
382-
file: StaticString = #file, line: UInt = #line) {
383-
switch result {
384-
case let .success(value):
385-
XCTAssertNotNil(value, file: file, line: line)
386-
case let .failure(error):
387-
XCTFail("Unexpected error \(error)")
388-
}
389-
}
390355
}
391356
#endif

FirebaseStorageSwift/Tests/Integration/StorageIntegrationCommon.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ class StorageIntegrationCommon: XCTestCase {
9999
}
100100

101101
private func waitForExpectations() {
102-
let kFIRStorageIntegrationTestTimeout = 60.0
103-
waitForExpectations(timeout: kFIRStorageIntegrationTestTimeout,
102+
let kTestTimeout = 60.0
103+
waitForExpectations(timeout: kTestTimeout,
104104
handler: { (error) -> Void in
105105
if let error = error {
106106
print(error)

0 commit comments

Comments
 (0)