From 73166724a9e17e3baff81af0f6ae8d6736fe0c35 Mon Sep 17 00:00:00 2001 From: dajiaohuang <108231307+dajiaohuang@users.noreply.github.com> Date: Fri, 2 Oct 2026 01:58:13 +0800 Subject: [PATCH 1/3] Fix Storage task resume setup races --- FirebaseStorage/CHANGELOG.md | 2 + .../Internal/StorageFetcherService.swift | 8 + .../Sources/StorageDownloadTask.swift | 35 +++- FirebaseStorage/Sources/StorageTask.swift | 3 + .../Sources/StorageUploadTask.swift | 24 +++ .../Tests/Unit/StorageTaskEnqueueTests.swift | 191 ++++++++++++++++++ 6 files changed, 258 insertions(+), 5 deletions(-) create mode 100644 FirebaseStorage/Tests/Unit/StorageTaskEnqueueTests.swift diff --git a/FirebaseStorage/CHANGELOG.md b/FirebaseStorage/CHANGELOG.md index 441516611b9..3dfe9a51102 100644 --- a/FirebaseStorage/CHANGELOG.md +++ b/FirebaseStorage/CHANGELOG.md @@ -1,4 +1,6 @@ # 12.19.0 +- [fixed] Prevented duplicate upload and download requests when resuming a task before its initial + request setup has completed. - [fixed] Fixed an issue where constructing a download URL could fail or produce an invalid path if the storage bucket name contained special characters. (#16529) - [changed] Requests directed to the local emulator over a non-loopback HTTP connection diff --git a/FirebaseStorage/Sources/Internal/StorageFetcherService.swift b/FirebaseStorage/Sources/Internal/StorageFetcherService.swift index 63af8e2fe2e..0bb0133e207 100644 --- a/FirebaseStorage/Sources/Internal/StorageFetcherService.swift +++ b/FirebaseStorage/Sources/Internal/StorageFetcherService.swift @@ -26,7 +26,11 @@ actor StorageFetcherService { private var _fetcherService: GTMSessionFetcherService? + // Allows unit tests to hold service setup across a task pause/resume transition. + private var serviceWaiterForTesting: (@Sendable () async -> Void)? + func service(_ storage: Storage) async -> GTMSessionFetcherService { + await serviceWaiterForTesting?() if let _fetcherService { return _fetcherService } @@ -77,6 +81,10 @@ actor StorageFetcherService { } } + func updateServiceWaiterForTesting(_ waiter: (@Sendable () async -> Void)?) { + serviceWaiterForTesting = waiter + } + private var testBlock: GTMSessionFetcherTestBlock? private var retryWhenOffline: GTMSessionFetcherRetryBlock = { diff --git a/FirebaseStorage/Sources/StorageDownloadTask.swift b/FirebaseStorage/Sources/StorageDownloadTask.swift index 474031f1f1d..01fdf755eef 100644 --- a/FirebaseStorage/Sources/StorageDownloadTask.swift +++ b/FirebaseStorage/Sources/StorageDownloadTask.swift @@ -38,8 +38,14 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u * Prepares a task and begins execution. */ @objc open func enqueue() { + let generation = stateLock.withLock { () -> UInt64? in + guard state == .unknown, fetcher == nil else { return nil } + enqueueGeneration &+= 1 + return enqueueGeneration + } + guard let generation else { return } Task { - await enqueueImplementation() + await enqueueImplementation(generation: generation) } } @@ -97,12 +103,14 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u @objc open func resume() { var downloadDataToResume: Data? var snapshotToFire: StorageTaskSnapshot? + var generation: UInt64 = 0 let shouldReturn1 = stateLock.withLock { () -> Bool in - guard state == .unknown || state == .queueing || state == .pausing || state == .paused || - state == .progress else { + guard state == .unknown || state == .queueing || state == .pausing || state == .paused else { return true } state = .resuming + enqueueGeneration &+= 1 + generation = enqueueGeneration downloadDataToResume = downloadData snapshotToFire = snapshotUnderLock() return false @@ -121,12 +129,16 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u if shouldEnqueue { Task { - await self.enqueueImplementation(resumeWith: downloadDataToResume) + await self.enqueueImplementation(resumeWith: downloadDataToResume, generation: generation) } } } private var fetcher: GTMSessionFetcher? + private var enqueueGeneration: UInt64 = 0 + var hasFetcherForTesting: Bool { + stateLock.withLock { fetcher != nil } + } var downloadData: Data? // Hold completion in object to force it to be retained until completion block is called. var completionData: ((Data?, Error?) -> Void)? @@ -144,8 +156,10 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u self.fetcher?.stopFetching() } - private func enqueueImplementation(resumeWith resumeData: Data? = nil) async { + private func enqueueImplementation(resumeWith resumeData: Data? = nil, + generation: UInt64) async { let shouldProceed = stateLock.withLock { () -> Bool in + guard generation == enqueueGeneration else { return false } guard state == .unknown || state == .queueing || state == .resuming || state == .running || state == .progress else { return false @@ -168,6 +182,10 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u fetcher.comment = "Resuming DownloadTask" } else { let fetcherService = await StorageFetcherService.shared.service(reference.storage) + guard stateLock.withLock({ generation == enqueueGeneration }) else { + setupDiscardedHandlerForTesting?() + return + } fetcher = fetcherService.fetcher(with: request) fetcher.comment = "Starting DownloadTask" @@ -183,6 +201,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u guard let self = self else { return } var snapshotToFire: StorageTaskSnapshot? self.stateLock.withLock { + guard generation == self.enqueueGeneration else { return } guard self.state == .unknown || self.state == .queueing || self.state == .resuming || self.state == .running || self.state == .progress else { return @@ -197,6 +216,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u } self.stateLock.withLock { + guard generation == self.enqueueGeneration else { return } guard self.state == .unknown || self.state == .queueing || self.state == .resuming || self.state == .running || self.state == .progress else { return @@ -211,6 +231,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u guard let self = self, let fetcher = fetcher else { return } var snapshotToFire: StorageTaskSnapshot? self.stateLock.withLock { + guard generation == self.enqueueGeneration else { return } guard self.state == .unknown || self.state == .queueing || self.state == .resuming || self.state == .running || self.state == .progress else { return @@ -227,6 +248,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u } self.stateLock.withLock { + guard generation == self.enqueueGeneration else { return } guard self.state == .unknown || self.state == .queueing || self.state == .resuming || self.state == .running || self.state == .progress else { return @@ -237,6 +259,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u } var isPausing = false let shouldContinue = stateLock.withLock { () -> Bool in + guard generation == enqueueGeneration else { return false } if state == .cancelled || state == .pausing || state == .paused { isPausing = state == .pausing return false @@ -274,6 +297,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u var successSnapshot: StorageTaskSnapshot? stateLock.withLock { + guard generation == enqueueGeneration else { return } if state == .cancelled { return } state = .progress @@ -296,6 +320,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u var failureSnapshot: StorageTaskSnapshot? stateLock.withLock { + guard generation == enqueueGeneration else { return } if state == .cancelled || state == .paused || state == .pausing { return } state = .progress diff --git a/FirebaseStorage/Sources/StorageTask.swift b/FirebaseStorage/Sources/StorageTask.swift index 75d0c0065d3..ed4bfb41a6e 100644 --- a/FirebaseStorage/Sources/StorageTask.swift +++ b/FirebaseStorage/Sources/StorageTask.swift @@ -91,6 +91,9 @@ import Foundation let baseRequest: URLRequest + // A narrow synchronization point for tests of task setup cancellation. + var setupDiscardedHandlerForTesting: (@Sendable () -> Void)? + init(reference: StorageReference, queue: DispatchQueue) { self.reference = reference diff --git a/FirebaseStorage/Sources/StorageUploadTask.swift b/FirebaseStorage/Sources/StorageUploadTask.swift index 49849ed6312..a1d75b9d5ad 100644 --- a/FirebaseStorage/Sources/StorageUploadTask.swift +++ b/FirebaseStorage/Sources/StorageUploadTask.swift @@ -44,11 +44,22 @@ import Foundation * Prepares a task and begins execution. */ @objc open func enqueue() { + let generation = stateLock.withLock { () -> UInt64? in + guard uploadFetcher == nil, + state == .unknown || state == .resuming || state == .running else { + return nil + } + enqueueGeneration &+= 1 + return enqueueGeneration + } + guard let generation else { return } + // Capturing self so that the upload is done whether or not there is a callback. dispatchQueue.async { [self] in let contentValidationError = self.contentUploadError() var failureSnapshot: StorageTaskSnapshot? let shouldProceed = stateLock.withLock { () -> Bool in + guard generation == enqueueGeneration else { return false } guard state == .unknown || state == .queueing || state == .resuming || state == .running || state == .progress else { return false @@ -74,6 +85,10 @@ import Foundation Task { let fetcherService = await StorageFetcherService.shared.service(reference.storage) + guard stateLock.withLock({ generation == enqueueGeneration }) else { + setupDiscardedHandlerForTesting?() + return + } var request = self.baseRequest request.httpMethod = "POST" request.timeoutInterval = self.reference.storage.maxUploadRetryTime @@ -127,6 +142,7 @@ import Foundation guard let self = self else { return } var snapshotToFire: StorageTaskSnapshot? self.stateLock.withLock { + guard generation == self.enqueueGeneration else { return } guard self.state == .unknown || self.state == .queueing || self.state == .resuming || self.state == .running || self.state == .progress else { return @@ -142,6 +158,7 @@ import Foundation } self.stateLock.withLock { + guard generation == self.enqueueGeneration else { return } guard self.state == .unknown || self.state == .queueing || self.state == .resuming || self.state == .running || self.state == .progress else { return @@ -152,6 +169,7 @@ import Foundation // Process fetches var isPaused = false let shouldContinue = self.stateLock.withLock { () -> Bool in + guard generation == self.enqueueGeneration else { return false } if self.state == .cancelled || self.state == .pausing || self.state == .paused { isPaused = self.state == .paused || self.state == .pausing return false @@ -175,6 +193,7 @@ import Foundation var failureSnapshot: StorageTaskSnapshot? self.stateLock.withLock { + guard generation == self.enqueueGeneration else { return } if self.state == .cancelled { return } self.state = .progress @@ -207,6 +226,7 @@ import Foundation var failureSnapshot: StorageTaskSnapshot? self.stateLock.withLock { + guard generation == self.enqueueGeneration else { return } if self.state == .cancelled || self.state == .paused || self .state == .pausing { return } @@ -320,6 +340,10 @@ import Foundation } private var uploadFetcher: GTMSessionUploadFetcher? + private var enqueueGeneration: UInt64 = 0 + var hasFetcherForTesting: Bool { + stateLock.withLock { uploadFetcher != nil } + } private var uploadMetadata: StorageMetadata private var uploadData: Data? // Hold completion in object to force it to be retained until completion block is called. diff --git a/FirebaseStorage/Tests/Unit/StorageTaskEnqueueTests.swift b/FirebaseStorage/Tests/Unit/StorageTaskEnqueueTests.swift new file mode 100644 index 00000000000..2e0f8fc1349 --- /dev/null +++ b/FirebaseStorage/Tests/Unit/StorageTaskEnqueueTests.swift @@ -0,0 +1,191 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +@testable import FirebaseStorage +import Foundation +#if COCOAPODS + import GTMSessionFetcher +#else + import GTMSessionFetcherCore +#endif +import XCTest + +class StorageTaskEnqueueTests: StorageTestHelpers { + func testUploadResumeDiscardsEarlierSetup() async throws { + let serviceGate = FirstCallServiceGate() + let discardedSetup = AsyncSignal() + let requestCount = FetcherRequestCounter() + let requestStarted = expectation(description: "Resumed upload starts one request") + let testBlock: GTMSessionFetcherTestBlock = { fetcher, response in + if requestCount.increment() == 1 { + requestStarted.fulfill() + } + guard let responseURL = fetcher.request?.url else { + XCTFail("Fetcher request should have a URL") + return + } + response( + HTTPURLResponse( + url: responseURL, + statusCode: 200, + httpVersion: "HTTP/1.1", + headerFields: nil + ), + Data("{}".utf8), + nil + ) + } + await StorageFetcherService.shared.updateTestBlock(testBlock) + await StorageFetcherService.shared.updateServiceWaiterForTesting { + await serviceGate.waitForFirstCall() + } + addTeardownBlock { + await StorageFetcherService.shared.updateTestBlock(nil) + await StorageFetcherService.shared.updateServiceWaiterForTesting(nil) + } + + let task = rootReference().putData(Data("upload".utf8)) + task.setupDiscardedHandlerForTesting = { + Task { await discardedSetup.signal() } + } + await serviceGate.waitUntilFirstCallEntered() + + task.pause() + task.resume() + await fulfillment(of: [requestStarted], timeout: 10) + XCTAssertTrue(task.hasFetcherForTesting) + + await serviceGate.releaseFirstCall() + await discardedSetup.wait() + XCTAssertEqual(requestCount.value, 1) + } + + func testDownloadResumeDiscardsEarlierSetup() async throws { + let serviceGate = FirstCallServiceGate() + let discardedSetup = AsyncSignal() + let requestCount = FetcherRequestCounter() + let requestStarted = expectation(description: "Resumed download starts one request") + let testBlock: GTMSessionFetcherTestBlock = { fetcher, response in + if requestCount.increment() == 1 { + requestStarted.fulfill() + } + guard let responseURL = fetcher.request?.url else { + XCTFail("Fetcher request should have a URL") + return + } + response( + HTTPURLResponse( + url: responseURL, + statusCode: 200, + httpVersion: "HTTP/1.1", + headerFields: nil + ), + Data("download".utf8), + nil + ) + } + await StorageFetcherService.shared.updateTestBlock(testBlock) + await StorageFetcherService.shared.updateServiceWaiterForTesting { + await serviceGate.waitForFirstCall() + } + addTeardownBlock { + await StorageFetcherService.shared.updateTestBlock(nil) + await StorageFetcherService.shared.updateServiceWaiterForTesting(nil) + } + + let reference = rootReference() + let task = StorageDownloadTask( + reference: reference, + queue: reference.storage.dispatchQueue, + file: nil + ) + task.setupDiscardedHandlerForTesting = { + Task { await discardedSetup.signal() } + } + task.enqueue() + await serviceGate.waitUntilFirstCallEntered() + + task.pause() + task.resume() + await fulfillment(of: [requestStarted], timeout: 10) + XCTAssertTrue(task.hasFetcherForTesting) + + await serviceGate.releaseFirstCall() + await discardedSetup.wait() + XCTAssertEqual(requestCount.value, 1) + } +} + +private actor FirstCallServiceGate { + private var callCount = 0 + private var firstCallEntered = false + private var entryContinuation: CheckedContinuation? + private var releaseContinuation: CheckedContinuation? + + func waitForFirstCall() async { + callCount += 1 + guard callCount == 1 else { return } + await withCheckedContinuation { continuation in + firstCallEntered = true + releaseContinuation = continuation + entryContinuation?.resume() + entryContinuation = nil + } + } + + func waitUntilFirstCallEntered() async { + guard !firstCallEntered else { return } + await withCheckedContinuation { continuation in + entryContinuation = continuation + } + } + + func releaseFirstCall() { + releaseContinuation?.resume() + releaseContinuation = nil + } +} + +private actor AsyncSignal { + private var isSignaled = false + private var continuation: CheckedContinuation? + + func signal() { + isSignaled = true + continuation?.resume() + continuation = nil + } + + func wait() async { + guard !isSignaled else { return } + await withCheckedContinuation { continuation = $0 } + } +} + +private final class FetcherRequestCounter: @unchecked Sendable { + private let lock = NSLock() + private var count = 0 + + @discardableResult + func increment() -> Int { + lock.withLock { + count += 1 + return count + } + } + + var value: Int { + lock.withLock { count } + } +} From a92be3fd6ab7061d8342c44a7dc3b52535634bd7 Mon Sep 17 00:00:00 2001 From: dajiaohuang <108231307+dajiaohuang@users.noreply.github.com> Date: Fri, 2 Oct 2026 15:20:26 +0800 Subject: [PATCH 2/3] Format Storage task sources --- FirebaseStorage/Sources/StorageDownloadTask.swift | 1 + FirebaseStorage/Sources/StorageUploadTask.swift | 1 + 2 files changed, 2 insertions(+) diff --git a/FirebaseStorage/Sources/StorageDownloadTask.swift b/FirebaseStorage/Sources/StorageDownloadTask.swift index 01fdf755eef..a347a35eade 100644 --- a/FirebaseStorage/Sources/StorageDownloadTask.swift +++ b/FirebaseStorage/Sources/StorageDownloadTask.swift @@ -139,6 +139,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u var hasFetcherForTesting: Bool { stateLock.withLock { fetcher != nil } } + var downloadData: Data? // Hold completion in object to force it to be retained until completion block is called. var completionData: ((Data?, Error?) -> Void)? diff --git a/FirebaseStorage/Sources/StorageUploadTask.swift b/FirebaseStorage/Sources/StorageUploadTask.swift index a1d75b9d5ad..59cd9fe1057 100644 --- a/FirebaseStorage/Sources/StorageUploadTask.swift +++ b/FirebaseStorage/Sources/StorageUploadTask.swift @@ -344,6 +344,7 @@ import Foundation var hasFetcherForTesting: Bool { stateLock.withLock { uploadFetcher != nil } } + private var uploadMetadata: StorageMetadata private var uploadData: Data? // Hold completion in object to force it to be retained until completion block is called. From 3b0555b6bb19cb9c55cecc026210263e92196b4b Mon Sep 17 00:00:00 2001 From: dajiaohuang <108231307+dajiaohuang@users.noreply.github.com> Date: Sat, 3 Oct 2026 01:14:41 +0800 Subject: [PATCH 3/3] Cover immediate Storage resume and bound setup regressions --- FirebaseStorage/CHANGELOG.md | 6 +- .../Sources/StorageDownloadTask.swift | 8 +- FirebaseStorage/Sources/StorageTask.swift | 11 +- .../Sources/StorageUploadTask.swift | 5 +- .../Tests/Unit/StorageTaskEnqueueTests.swift | 163 +++++++++++++----- 5 files changed, 144 insertions(+), 49 deletions(-) diff --git a/FirebaseStorage/CHANGELOG.md b/FirebaseStorage/CHANGELOG.md index 3dfe9a51102..74060926718 100644 --- a/FirebaseStorage/CHANGELOG.md +++ b/FirebaseStorage/CHANGELOG.md @@ -1,6 +1,8 @@ -# 12.19.0 +# Unreleased - [fixed] Prevented duplicate upload and download requests when resuming a task before its initial - request setup has completed. + request setup has completed. (#16771) + +# 12.19.0 - [fixed] Fixed an issue where constructing a download URL could fail or produce an invalid path if the storage bucket name contained special characters. (#16529) - [changed] Requests directed to the local emulator over a non-loopback HTTP connection diff --git a/FirebaseStorage/Sources/StorageDownloadTask.swift b/FirebaseStorage/Sources/StorageDownloadTask.swift index a347a35eade..460c1524224 100644 --- a/FirebaseStorage/Sources/StorageDownloadTask.swift +++ b/FirebaseStorage/Sources/StorageDownloadTask.swift @@ -168,7 +168,10 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u state = .queueing return true } - if !shouldProceed { return } + if !shouldProceed { + notifySetupDiscardedForTesting() + return + } var request = baseRequest request.httpMethod = "GET" @@ -184,7 +187,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u } else { let fetcherService = await StorageFetcherService.shared.service(reference.storage) guard stateLock.withLock({ generation == enqueueGeneration }) else { - setupDiscardedHandlerForTesting?() + notifySetupDiscardedForTesting() return } @@ -270,6 +273,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement, @u return true } if !shouldContinue { + notifySetupDiscardedForTesting() if isPausing { fetcher.resumeDataBlock = { [weak self] (data: Data) in guard let self = self else { return } diff --git a/FirebaseStorage/Sources/StorageTask.swift b/FirebaseStorage/Sources/StorageTask.swift index ed4bfb41a6e..39c22c4815a 100644 --- a/FirebaseStorage/Sources/StorageTask.swift +++ b/FirebaseStorage/Sources/StorageTask.swift @@ -92,7 +92,16 @@ import Foundation let baseRequest: URLRequest // A narrow synchronization point for tests of task setup cancellation. - var setupDiscardedHandlerForTesting: (@Sendable () -> Void)? + private var setupDiscardedHandler: (@Sendable () -> Void)? + var setupDiscardedHandlerForTesting: (@Sendable () -> Void)? { + get { stateLock.withLock { setupDiscardedHandler } } + set { stateLock.withLock { setupDiscardedHandler = newValue } } + } + + func notifySetupDiscardedForTesting() { + let handler = stateLock.withLock { setupDiscardedHandler } + handler?() + } init(reference: StorageReference, queue: DispatchQueue) { diff --git a/FirebaseStorage/Sources/StorageUploadTask.swift b/FirebaseStorage/Sources/StorageUploadTask.swift index 59cd9fe1057..fbfc55151fb 100644 --- a/FirebaseStorage/Sources/StorageUploadTask.swift +++ b/FirebaseStorage/Sources/StorageUploadTask.swift @@ -76,6 +76,8 @@ import Foundation if !shouldProceed { if let failureSnapshot { self.finishTaskWithStatus(status: .failure, snapshot: failureSnapshot) + } else { + notifySetupDiscardedForTesting() } return } @@ -86,7 +88,7 @@ import Foundation Task { let fetcherService = await StorageFetcherService.shared.service(reference.storage) guard stateLock.withLock({ generation == enqueueGeneration }) else { - setupDiscardedHandlerForTesting?() + notifySetupDiscardedForTesting() return } var request = self.baseRequest @@ -179,6 +181,7 @@ import Foundation return true } if !shouldContinue { + notifySetupDiscardedForTesting() if isPaused { uploadFetcher.pauseFetching() } else { diff --git a/FirebaseStorage/Tests/Unit/StorageTaskEnqueueTests.swift b/FirebaseStorage/Tests/Unit/StorageTaskEnqueueTests.swift index 2e0f8fc1349..51accdc942b 100644 --- a/FirebaseStorage/Tests/Unit/StorageTaskEnqueueTests.swift +++ b/FirebaseStorage/Tests/Unit/StorageTaskEnqueueTests.swift @@ -22,9 +22,76 @@ import Foundation import XCTest class StorageTaskEnqueueTests: StorageTestHelpers { + func testDownloadResumeDuringProgressDoesNotStartAnotherRequest() async throws { + let firstCallEntered = expectation(description: "Download setup enters service") + let serviceGate = FirstCallServiceGate(firstCallEntered: firstCallEntered) + let responseLatch = FetcherResponseLatch() + let requestCount = FetcherRequestCounter() + let progressCount = FetcherRequestCounter() + let progressObserved = expectation(description: "Resume from download progress") + let completed = expectation(description: "Download completes") + let testBlock: GTMSessionFetcherTestBlock = { fetcher, response in + requestCount.increment() + guard let url = fetcher.request?.url else { + XCTFail("Fetcher request should have a URL") + return + } + responseLatch.install { + response( + HTTPURLResponse(url: url, statusCode: 200, httpVersion: "HTTP/1.1", headerFields: nil), + Data("download".utf8), + nil + ) + } + guard let receivedProgress = fetcher.receivedProgressBlock else { + XCTFail("Download should install its received-progress callback") + responseLatch.release() + return + } + receivedProgress(1, 1) + } + await StorageFetcherService.shared.updateTestBlock(testBlock) + await StorageFetcherService.shared.updateServiceWaiterForTesting { + await serviceGate.waitForFirstCall() + } + addTeardownBlock { + responseLatch.release() + await serviceGate.releaseFirstCall() + await StorageFetcherService.shared.updateTestBlock(nil) + await StorageFetcherService.shared.updateServiceWaiterForTesting(nil) + } + let task = rootReference().child("object").getData(maxSize: 1024) { data, error in + XCTAssertNil(error) + XCTAssertEqual(data, Data("download".utf8)) + completed.fulfill() + } + task.observe(.progress) { _ in + guard progressCount.increment() == 1 else { return } + // Make the transient progress phase deterministic before completion is released. + task.stateLock.withLock { task.state = .progress } + task.resume() + XCTAssertTrue(task.hasFetcherForTesting) + progressObserved.fulfill() + responseLatch.release() + } + await fulfillment(of: [firstCallEntered], timeout: 10) + await serviceGate.releaseFirstCall() + await fulfillment(of: [progressObserved, completed], timeout: 10) + XCTAssertEqual(requestCount.value, 1) + } + func testUploadResumeDiscardsEarlierSetup() async throws { - let serviceGate = FirstCallServiceGate() - let discardedSetup = AsyncSignal() + try await uploadResumeDiscardsEarlierSetup(pauseFirst: true) + } + + func testImmediateUploadResumeDiscardsEarlierSetup() async throws { + try await uploadResumeDiscardsEarlierSetup(pauseFirst: false) + } + + private func uploadResumeDiscardsEarlierSetup(pauseFirst: Bool) async throws { + let firstCallEntered = expectation(description: "Initial upload setup enters service") + let serviceGate = FirstCallServiceGate(firstCallEntered: firstCallEntered) + let discardedSetup = expectation(description: "Initial upload setup discarded") let requestCount = FetcherRequestCounter() let requestStarted = expectation(description: "Resumed upload starts one request") let testBlock: GTMSessionFetcherTestBlock = { fetcher, response in @@ -51,29 +118,39 @@ class StorageTaskEnqueueTests: StorageTestHelpers { await serviceGate.waitForFirstCall() } addTeardownBlock { + await serviceGate.releaseFirstCall() await StorageFetcherService.shared.updateTestBlock(nil) await StorageFetcherService.shared.updateServiceWaiterForTesting(nil) } - let task = rootReference().putData(Data("upload".utf8)) + let task = rootReference().child("object").putData(Data("upload".utf8)) task.setupDiscardedHandlerForTesting = { - Task { await discardedSetup.signal() } + discardedSetup.fulfill() } - await serviceGate.waitUntilFirstCallEntered() + await fulfillment(of: [firstCallEntered], timeout: 10) - task.pause() + if pauseFirst { task.pause() } task.resume() await fulfillment(of: [requestStarted], timeout: 10) XCTAssertTrue(task.hasFetcherForTesting) await serviceGate.releaseFirstCall() - await discardedSetup.wait() + await fulfillment(of: [discardedSetup], timeout: 10) XCTAssertEqual(requestCount.value, 1) } func testDownloadResumeDiscardsEarlierSetup() async throws { - let serviceGate = FirstCallServiceGate() - let discardedSetup = AsyncSignal() + try await downloadResumeDiscardsEarlierSetup(pauseFirst: true) + } + + func testImmediateDownloadResumeDiscardsEarlierSetup() async throws { + try await downloadResumeDiscardsEarlierSetup(pauseFirst: false) + } + + private func downloadResumeDiscardsEarlierSetup(pauseFirst: Bool) async throws { + let firstCallEntered = expectation(description: "Initial download setup enters service") + let serviceGate = FirstCallServiceGate(firstCallEntered: firstCallEntered) + let discardedSetup = expectation(description: "Initial download setup discarded") let requestCount = FetcherRequestCounter() let requestStarted = expectation(description: "Resumed download starts one request") let testBlock: GTMSessionFetcherTestBlock = { fetcher, response in @@ -100,76 +177,76 @@ class StorageTaskEnqueueTests: StorageTestHelpers { await serviceGate.waitForFirstCall() } addTeardownBlock { + await serviceGate.releaseFirstCall() await StorageFetcherService.shared.updateTestBlock(nil) await StorageFetcherService.shared.updateServiceWaiterForTesting(nil) } - let reference = rootReference() - let task = StorageDownloadTask( - reference: reference, - queue: reference.storage.dispatchQueue, - file: nil - ) + let task = rootReference().child("object").getData(maxSize: 1024) { _, _ in } task.setupDiscardedHandlerForTesting = { - Task { await discardedSetup.signal() } + discardedSetup.fulfill() } - task.enqueue() - await serviceGate.waitUntilFirstCallEntered() + await fulfillment(of: [firstCallEntered], timeout: 10) - task.pause() + if pauseFirst { task.pause() } task.resume() await fulfillment(of: [requestStarted], timeout: 10) XCTAssertTrue(task.hasFetcherForTesting) await serviceGate.releaseFirstCall() - await discardedSetup.wait() + await fulfillment(of: [discardedSetup], timeout: 10) XCTAssertEqual(requestCount.value, 1) } } private actor FirstCallServiceGate { + let firstCallEntered: XCTestExpectation private var callCount = 0 - private var firstCallEntered = false - private var entryContinuation: CheckedContinuation? + private var isReleased = false private var releaseContinuation: CheckedContinuation? + init(firstCallEntered: XCTestExpectation) { + self.firstCallEntered = firstCallEntered + } + func waitForFirstCall() async { callCount += 1 - guard callCount == 1 else { return } + guard callCount == 1, !isReleased else { return } await withCheckedContinuation { continuation in - firstCallEntered = true releaseContinuation = continuation - entryContinuation?.resume() - entryContinuation = nil - } - } - - func waitUntilFirstCallEntered() async { - guard !firstCallEntered else { return } - await withCheckedContinuation { continuation in - entryContinuation = continuation + firstCallEntered.fulfill() } } func releaseFirstCall() { + isReleased = true releaseContinuation?.resume() releaseContinuation = nil } } -private actor AsyncSignal { - private var isSignaled = false - private var continuation: CheckedContinuation? +private final class FetcherResponseLatch: @unchecked Sendable { + private let lock = NSLock() + private var response: (() -> Void)? + private var isReleased = false - func signal() { - isSignaled = true - continuation?.resume() - continuation = nil + func install(_ response: @escaping () -> Void) { + let shouldRun = lock.withLock { + if isReleased { return true } + self.response = response + return false + } + if shouldRun { response() } } - func wait() async { - guard !isSignaled else { return } - await withCheckedContinuation { continuation = $0 } + func release() { + let response = lock.withLock { + isReleased = true + let response = self.response + self.response = nil + return response + } + response?() } }