diff --git a/.github/workflows/storage.yml b/.github/workflows/storage.yml index 9f311367c3c..14577436e56 100644 --- a/.github/workflows/storage.yml +++ b/.github/workflows/storage.yml @@ -1,184 +1,183 @@ -# TODO(Swift 6): Re-enable these tests. -# name: storage +name: storage -# on: -# workflow_dispatch: -# pull_request: -# paths: -# - 'FirebaseStorage**' -# - 'FirebaseAuth/Interop/*.h' -# - '.github/workflows/storage.yml' -# # Rebuild on Ruby infrastructure changes. -# - 'Gemfile*' -# schedule: -# # Run every day at 12am (PST) - cron uses UTC times -# - cron: '0 8 * * *' +on: + workflow_dispatch: + pull_request: + paths: + - 'FirebaseStorage**' + - 'FirebaseAuth/Interop/*.h' + - '.github/workflows/storage.yml' + # Rebuild on Ruby infrastructure changes. + - 'Gemfile*' + schedule: + # Run every day at 12am (PST) - cron uses UTC times + - cron: '0 8 * * *' -# concurrency: -# group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} -# cancel-in-progress: true +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} + cancel-in-progress: true -# jobs: -# spm: -# uses: ./.github/workflows/common.yml -# with: -# target: FirebaseStorageUnit +jobs: + spm: + uses: ./.github/workflows/common.yml + with: + target: FirebaseStorageUnit -# catalyst: -# uses: ./.github/workflows/common_catalyst.yml -# with: -# product: FirebaseStorage -# target: FirebaseStorage-Unit-unit + catalyst: + uses: ./.github/workflows/common_catalyst.yml + with: + product: FirebaseStorage + target: FirebaseStorage-Unit-unit -# storage-integration-tests: -# # Don't run on private repo unless it is a PR. -# if: (github.repository == 'Firebase/firebase-ios-sdk' && github.event_name == 'schedule') || github.event_name == 'pull_request' -# strategy: -# matrix: -# language: [Swift, ObjC] -# include: -# - os: macos-15 -# xcode: Xcode_16.3 -# env: -# plist_secret: ${{ secrets.GHASecretsGPGPassphrase1 }} -# runs-on: ${{ matrix.os }} -# steps: -# - uses: actions/checkout@v4 -# - uses: mikehardy/buildcache-action@c87cea0ccd718971d6cc39e672c4f26815b6c126 -# with: -# cache_key: integration${{ matrix.os }} -# - uses: ruby/setup-ruby@354a1ad156761f5ee2b7b13fa8e09943a5e8d252 # v1 -# - name: Setup Bundler -# run: scripts/setup_bundler.sh -# - name: Install xcpretty -# run: gem install xcpretty -# - name: Install Secret GoogleService-Info.plist -# run: scripts/decrypt_gha_secret.sh scripts/gha-encrypted/storage-db-plist.gpg \ -# FirebaseStorage/Tests/Integration/Resources/GoogleService-Info.plist "$plist_secret" -# - name: Install Credentials.h -# run: scripts/decrypt_gha_secret.sh scripts/gha-encrypted/Storage/Credentials.h.gpg \ -# FirebaseStorage/Tests/ObjCIntegration/Credentials.h "$plist_secret" -# - name: Install Credentials.swift -# run: | -# scripts/decrypt_gha_secret.sh scripts/gha-encrypted/Storage/Credentials.swift.gpg \ -# FirebaseStorage/Tests/Integration/Credentials.swift "$plist_secret" -# - name: Xcode -# run: sudo xcode-select -s /Applications/${{ matrix.xcode }}.app/Contents/Developer -# - uses: nick-fields/retry@ce71cc2ab81d554ebbe88c79ab5975992d79ba08 # v3 -# with: -# timeout_minutes: 120 -# max_attempts: 3 -# retry_on: error -# retry_wait_seconds: 120 -# command: ([ -z $plist_secret ] || scripts/build.sh Storage${{ matrix.language }} all) + storage-integration-tests: + # Don't run on private repo unless it is a PR. + if: (github.repository == 'Firebase/firebase-ios-sdk' && github.event_name == 'schedule') || github.event_name == 'pull_request' + strategy: + matrix: + language: [Swift, ObjC] + include: + - os: macos-15 + xcode: Xcode_16.3 + env: + plist_secret: ${{ secrets.GHASecretsGPGPassphrase1 }} + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + - uses: mikehardy/buildcache-action@c87cea0ccd718971d6cc39e672c4f26815b6c126 + with: + cache_key: integration${{ matrix.os }} + - uses: ruby/setup-ruby@354a1ad156761f5ee2b7b13fa8e09943a5e8d252 # v1 + - name: Setup Bundler + run: scripts/setup_bundler.sh + - name: Install xcpretty + run: gem install xcpretty + - name: Install Secret GoogleService-Info.plist + run: scripts/decrypt_gha_secret.sh scripts/gha-encrypted/storage-db-plist.gpg \ + FirebaseStorage/Tests/Integration/Resources/GoogleService-Info.plist "$plist_secret" + - name: Install Credentials.h + run: scripts/decrypt_gha_secret.sh scripts/gha-encrypted/Storage/Credentials.h.gpg \ + FirebaseStorage/Tests/ObjCIntegration/Credentials.h "$plist_secret" + - name: Install Credentials.swift + run: | + scripts/decrypt_gha_secret.sh scripts/gha-encrypted/Storage/Credentials.swift.gpg \ + FirebaseStorage/Tests/Integration/Credentials.swift "$plist_secret" + - name: Xcode + run: sudo xcode-select -s /Applications/${{ matrix.xcode }}.app/Contents/Developer + - uses: nick-fields/retry@ce71cc2ab81d554ebbe88c79ab5975992d79ba08 # v3 + with: + timeout_minutes: 120 + max_attempts: 3 + retry_on: error + retry_wait_seconds: 120 + command: ([ -z $plist_secret ] || scripts/build.sh Storage${{ matrix.language }} all) -# quickstart: -# # Don't run on private repo unless it is a PR. -# if: (github.repository == 'Firebase/firebase-ios-sdk' && github.event_name == 'schedule') || github.event_name == 'pull_request' -# # TODO: See #12399 and restore Objective-C testing for Xcode 15 if GHA is fixed. -# strategy: -# matrix: -# include: -# #- os: macos-13 -# # xcode: Xcode_14.2 # TODO: the legacy ObjC quickstart doesn't build with Xcode 15. -# - swift: swift -# os: macos-15 -# xcode: Xcode_16.2 -# env: -# plist_secret: ${{ secrets.GHASecretsGPGPassphrase1 }} -# signin_secret: ${{ secrets.GHASecretsGPGPassphrase1 }} -# LEGACY: true -# runs-on: ${{ matrix.os }} -# steps: -# - uses: actions/checkout@v4 -# - uses: ruby/setup-ruby@354a1ad156761f5ee2b7b13fa8e09943a5e8d252 # v1 -# - name: Setup quickstart -# run: scripts/setup_quickstart.sh storage -# - name: Install Secret GoogleService-Info.plist -# run: scripts/decrypt_gha_secret.sh scripts/gha-encrypted/qs-storage.plist.gpg \ -# quickstart-ios/storage/GoogleService-Info.plist "$plist_secret" -# - name: Xcode -# run: sudo xcode-select -s /Applications/${{ matrix.xcode }}.app/Contents/Developer -# - name: Test quickstart -# run: ([ -z $plist_secret ] || scripts/third_party/travis/retry.sh scripts/test_quickstart.sh Storage false ${{ matrix.swift }}) + quickstart: + # Don't run on private repo unless it is a PR. + if: (github.repository == 'Firebase/firebase-ios-sdk' && github.event_name == 'schedule') || github.event_name == 'pull_request' + # TODO: See #12399 and restore Objective-C testing for Xcode 15 if GHA is fixed. + strategy: + matrix: + include: + #- os: macos-13 + # xcode: Xcode_14.2 # TODO: the legacy ObjC quickstart doesn't build with Xcode 15. + - swift: swift + os: macos-15 + xcode: Xcode_16.2 + env: + plist_secret: ${{ secrets.GHASecretsGPGPassphrase1 }} + signin_secret: ${{ secrets.GHASecretsGPGPassphrase1 }} + LEGACY: true + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + - uses: ruby/setup-ruby@354a1ad156761f5ee2b7b13fa8e09943a5e8d252 # v1 + - name: Setup quickstart + run: scripts/setup_quickstart.sh storage + - name: Install Secret GoogleService-Info.plist + run: scripts/decrypt_gha_secret.sh scripts/gha-encrypted/qs-storage.plist.gpg \ + quickstart-ios/storage/GoogleService-Info.plist "$plist_secret" + - name: Xcode + run: sudo xcode-select -s /Applications/${{ matrix.xcode }}.app/Contents/Developer + - name: Test quickstart + run: ([ -z $plist_secret ] || scripts/third_party/travis/retry.sh scripts/test_quickstart.sh Storage false ${{ matrix.swift }}) -# quickstart-ftl-cron-only: -# # Don't run on private repo. -# if: github.repository == 'Firebase/firebase-ios-sdk' && github.event_name == 'schedule' -# env: -# plist_secret: ${{ secrets.GHASecretsGPGPassphrase1 }} -# signin_secret: ${{ secrets.GHASecretsGPGPassphrase1 }} -# LEGACY: true -# runs-on: macos-15 -# steps: -# - uses: actions/checkout@v4 -# - uses: ruby/setup-ruby@354a1ad156761f5ee2b7b13fa8e09943a5e8d252 # v1 -# - uses: actions/setup-python@v5 -# with: -# python-version: '3.11' -# - name: Setup quickstart -# run: scripts/setup_quickstart.sh storage -# - name: Install Secret GoogleService-Info.plist -# run: scripts/decrypt_gha_secret.sh scripts/gha-encrypted/qs-storage.plist.gpg \ -# quickstart-ios/storage/GoogleService-Info.plist "$plist_secret" -# # - name: Build objc quickstart -# # run: ([ -z $plist_secret ] || scripts/third_party/travis/retry.sh scripts/test_quickstart_ftl.sh Storage) -# - name: Build swift quickstart -# run: ([ -z $plist_secret ] || scripts/third_party/travis/retry.sh scripts/test_quickstart_ftl.sh Storage swift) -# - id: ftl_test -# uses: FirebaseExtended/github-actions/firebase-test-lab@v1.4 -# with: -# credentials_json: ${{ secrets.FIREBASE_SERVICE_ACCOUNT_CREDENTIALS }} -# testapp_dir: quickstart-ios/build-for-testing -# test_type: "xctest" + quickstart-ftl-cron-only: + # Don't run on private repo. + if: github.repository == 'Firebase/firebase-ios-sdk' && github.event_name == 'schedule' + env: + plist_secret: ${{ secrets.GHASecretsGPGPassphrase1 }} + signin_secret: ${{ secrets.GHASecretsGPGPassphrase1 }} + LEGACY: true + runs-on: macos-15 + steps: + - uses: actions/checkout@v4 + - uses: ruby/setup-ruby@354a1ad156761f5ee2b7b13fa8e09943a5e8d252 # v1 + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + - name: Setup quickstart + run: scripts/setup_quickstart.sh storage + - name: Install Secret GoogleService-Info.plist + run: scripts/decrypt_gha_secret.sh scripts/gha-encrypted/qs-storage.plist.gpg \ + quickstart-ios/storage/GoogleService-Info.plist "$plist_secret" + # - name: Build objc quickstart + # run: ([ -z $plist_secret ] || scripts/third_party/travis/retry.sh scripts/test_quickstart_ftl.sh Storage) + - name: Build swift quickstart + run: ([ -z $plist_secret ] || scripts/third_party/travis/retry.sh scripts/test_quickstart_ftl.sh Storage swift) + - id: ftl_test + uses: FirebaseExtended/github-actions/firebase-test-lab@v1.4 + with: + credentials_json: ${{ secrets.FIREBASE_SERVICE_ACCOUNT_CREDENTIALS }} + testapp_dir: quickstart-ios/build-for-testing + test_type: "xctest" -# pod-lib-lint: -# # Don't run on private repo unless it is a PR. -# if: (github.repository == 'Firebase/firebase-ios-sdk' && github.event_name == 'schedule') || github.event_name == 'pull_request' -# strategy: -# matrix: -# target: [ios, tvos, macos, watchos] -# build-env: -# - os: macos-15 -# xcode: Xcode_16.2 -# tests: --skip-tests -# - os: macos-15 -# xcode: Xcode_16.2 -# tests: --test-specs=unit -# runs-on: ${{ matrix.build-env.os }} -# steps: -# - uses: actions/checkout@v4 -# - uses: ruby/setup-ruby@354a1ad156761f5ee2b7b13fa8e09943a5e8d252 # v1 -# - name: Setup Bundler -# run: scripts/setup_bundler.sh -# - name: Xcodes -# run: ls -l /Applications/Xcode* -# - name: Xcode -# run: sudo xcode-select -s /Applications/${{ matrix.build-env.xcode }}.app/Contents/Developer -# - name: Build and test -# run: | -# scripts/third_party/travis/retry.sh scripts/pod_lib_lint.rb FirebaseStorage.podspec ${{ matrix.build-env.tests }} \ -# --platforms=${{ matrix.target }} + pod-lib-lint: + # Don't run on private repo unless it is a PR. + if: (github.repository == 'Firebase/firebase-ios-sdk' && github.event_name == 'schedule') || github.event_name == 'pull_request' + strategy: + matrix: + target: [ios, tvos, macos, watchos] + build-env: + - os: macos-15 + xcode: Xcode_16.2 + tests: --skip-tests + - os: macos-15 + xcode: Xcode_16.2 + tests: --test-specs=unit + runs-on: ${{ matrix.build-env.os }} + steps: + - uses: actions/checkout@v4 + - uses: ruby/setup-ruby@354a1ad156761f5ee2b7b13fa8e09943a5e8d252 # v1 + - name: Setup Bundler + run: scripts/setup_bundler.sh + - name: Xcodes + run: ls -l /Applications/Xcode* + - name: Xcode + run: sudo xcode-select -s /Applications/${{ matrix.build-env.xcode }}.app/Contents/Developer + - name: Build and test + run: | + scripts/third_party/travis/retry.sh scripts/pod_lib_lint.rb FirebaseStorage.podspec ${{ matrix.build-env.tests }} \ + --platforms=${{ matrix.target }} -# storage-cron-only: -# # Don't run on private repo. -# if: github.event_name == 'schedule' && github.repository == 'Firebase/firebase-ios-sdk' -# strategy: -# matrix: -# target: [ios, tvos, macos, watchos] -# build-env: -# - os: macos-14 -# xcode: Xcode_16.2 -# - os: macos-15 -# xcode: Xcode_16.2 -# runs-on: ${{ matrix.build-env.os }} -# needs: pod-lib-lint -# steps: -# - uses: actions/checkout@v4 -# - uses: ruby/setup-ruby@354a1ad156761f5ee2b7b13fa8e09943a5e8d252 # v1 -# - name: Setup Bundler -# run: scripts/setup_bundler.sh -# - name: Xcode -# run: sudo xcode-select -s /Applications/${{ matrix.build-env.xcode }}.app/Contents/Developer -# - name: PodLibLint Storage Cron -# run: scripts/third_party/travis/retry.sh scripts/pod_lib_lint.rb FirebaseStorage.podspec --platforms=${{ matrix.target }} --use-static-frameworks --skip-tests + storage-cron-only: + # Don't run on private repo. + if: github.event_name == 'schedule' && github.repository == 'Firebase/firebase-ios-sdk' + strategy: + matrix: + target: [ios, tvos, macos, watchos] + build-env: + - os: macos-14 + xcode: Xcode_16.2 + - os: macos-15 + xcode: Xcode_16.2 + runs-on: ${{ matrix.build-env.os }} + needs: pod-lib-lint + steps: + - uses: actions/checkout@v4 + - uses: ruby/setup-ruby@354a1ad156761f5ee2b7b13fa8e09943a5e8d252 # v1 + - name: Setup Bundler + run: scripts/setup_bundler.sh + - name: Xcode + run: sudo xcode-select -s /Applications/${{ matrix.build-env.xcode }}.app/Contents/Developer + - name: PodLibLint Storage Cron + run: scripts/third_party/travis/retry.sh scripts/pod_lib_lint.rb FirebaseStorage.podspec --platforms=${{ matrix.target }} --use-static-frameworks --skip-tests diff --git a/.ruby-version b/.ruby-version index fa376edcab7..71e447d5b6c 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -ruby-2.7 +ruby-3.1.4 diff --git a/FirebaseAuth/Sources/Swift/Auth/Auth.swift b/FirebaseAuth/Sources/Swift/Auth/Auth.swift index f88a1864990..c5093959b8d 100644 --- a/FirebaseAuth/Sources/Swift/Auth/Auth.swift +++ b/FirebaseAuth/Sources/Swift/Auth/Auth.swift @@ -2309,12 +2309,14 @@ extension Auth: AuthInterop { _ callback: @escaping @Sendable ( (T.Response?, Error?) -> Void )) { + // Avoids a compiler error saying `callback` can race. + let wrappedClosure = FIRNonisolatedUnsafe(initialState: callback) Task { do { let response = try await injectRecaptcha(request: request, action: action) - callback(response, nil) + wrappedClosure.state(response, nil) } catch { - callback(nil, error) + wrappedClosure.state(nil, error) } } } diff --git a/FirebaseCore/Internal/Sources/FIRAllocatedUnfairLock.swift b/FirebaseCore/Internal/Sources/FIRAllocatedUnfairLock.swift index ae52faefce6..5705f1cd424 100644 --- a/FirebaseCore/Internal/Sources/FIRAllocatedUnfairLock.swift +++ b/FirebaseCore/Internal/Sources/FIRAllocatedUnfairLock.swift @@ -64,3 +64,20 @@ public final class FIRAllocatedUnfairLock: @unchecked Sendable { lockPointer.deallocate() } } + +// This class is used to get around a limitation where local variables cannot be +// declared nonisolated for things like capture and mutation in escaping closures. +public final class FIRNonisolatedUnsafe: @unchecked Sendable { + public private(set) var state: State + + public init(initialState: State) { + state = initialState + } + + @discardableResult + public func withNonisolatedUnsafeState(_ body: (inout State) throws -> R) rethrows -> R { + let value: R + value = try body(&state) + return value + } +} diff --git a/FirebaseCore/Internal/Tests/Unit/HeartbeatStorageTests.swift b/FirebaseCore/Internal/Tests/Unit/HeartbeatStorageTests.swift index 826590b219c..c48cea653f7 100644 --- a/FirebaseCore/Internal/Tests/Unit/HeartbeatStorageTests.swift +++ b/FirebaseCore/Internal/Tests/Unit/HeartbeatStorageTests.swift @@ -407,7 +407,7 @@ class HeartbeatStorageTests: XCTestCase { final class WeakRefs: @unchecked Sendable { // Lock is used to synchronize `weakRefs` during concurrent access. private(set) var weakRefs = - FIRAllocatedUnfairLock<[WeakContainer]>(initialState: []) + FIRAllocatedUnfairLock<[WeakContainer]>(initialState: []) func append(_ weakRef: WeakContainer) { weakRefs.withLock { diff --git a/FirebaseStorage/Sources/Internal/StorageDeleteTask.swift b/FirebaseStorage/Sources/Internal/StorageDeleteTask.swift index d2917d18c49..ea486eef52c 100644 --- a/FirebaseStorage/Sources/Internal/StorageDeleteTask.swift +++ b/FirebaseStorage/Sources/Internal/StorageDeleteTask.swift @@ -25,7 +25,7 @@ import Foundation enum StorageDeleteTask { static func deleteTask(reference: StorageReference, queue: DispatchQueue, - completion: ((_: Data?, _: Error?) -> Void)?) { + completion: (@Sendable (_: Data?, _: Error?) -> Void)?) { StorageInternalTask(reference: reference, queue: queue, httpMethod: "DELETE", diff --git a/FirebaseStorage/Sources/Internal/StorageListTask.swift b/FirebaseStorage/Sources/Internal/StorageListTask.swift index 8d9369a6768..a1823d1dbf0 100644 --- a/FirebaseStorage/Sources/Internal/StorageListTask.swift +++ b/FirebaseStorage/Sources/Internal/StorageListTask.swift @@ -21,7 +21,7 @@ enum StorageListTask { queue: DispatchQueue, pageSize: Int64?, previousPageToken: String?, - completion: ((_: StorageListResult?, _: Error?) -> Void)?) { + completion: (@Sendable (_: StorageListResult?, _: Error?) -> Void)?) { var queryParams = [String: String]() let prefix = reference.fullPath diff --git a/FirebaseStorage/Sources/Internal/StorageTokenAuthorizer.swift b/FirebaseStorage/Sources/Internal/StorageTokenAuthorizer.swift index e35fb678eee..31edb297898 100644 --- a/FirebaseStorage/Sources/Internal/StorageTokenAuthorizer.swift +++ b/FirebaseStorage/Sources/Internal/StorageTokenAuthorizer.swift @@ -17,6 +17,7 @@ import Foundation @preconcurrency import FirebaseAppCheckInterop /* TODO: sendable */ import FirebaseAuthInterop import FirebaseCore +internal import FirebaseCoreInternal internal import FirebaseCoreExtension #if COCOAPODS @@ -27,16 +28,82 @@ internal import FirebaseCoreExtension @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) final class StorageTokenAuthorizer: NSObject, GTMSessionFetcherAuthorizer, Sendable { - func authorizeRequest(_ request: NSMutableURLRequest?, + func authorizeRequest(_ incomingRequest: NSMutableURLRequest?, completionHandler handler: @escaping @Sendable (Error?) -> Void) { - if let request = request { - Task { - do { - try await self._authorizeRequest(request) - handler(nil) - } catch { - handler(error) + guard let _request = incomingRequest else { + handler(nil) + return + } + let request = FIRNonisolatedUnsafe(initialState: _request) + + // Set version header on each request + let versionString = "ios/\(FirebaseVersion())" + + request.withNonisolatedUnsafeState { state in + state.setValue(versionString, forHTTPHeaderField: "x-firebase-storage-version") + + // Set GMP ID on each request + state.setValue(googleAppID, forHTTPHeaderField: "x-firebase-gmpid") + } + + // If there's no Auth to authorize the request, pass it back with just header changes. + guard let auth = auth else { + handler(nil) + return + } + + auth.getToken(forcingRefresh: false) { token, error in + let firebaseToken: String + + if let token { + firebaseToken = "Firebase \(token)" + } else if let error = error as? NSError { + var errorDictionary = error.userInfo + errorDictionary["ResponseErrorDomain"] = error.domain + errorDictionary["ResponseErrorCode"] = error.code + let wrappedError = StorageError.unauthenticated(serverError: errorDictionary) as Error + handler(wrappedError) + return + } else { + let underlyingError: [String: Any] + if let error = error { + underlyingError = [NSUnderlyingErrorKey: error] + } else { + underlyingError = [:] + } + let unknownError = StorageError.unknown( + message: "Auth token fetch returned no token or error: \(token ?? "nil")", + serverError: underlyingError + ) as Error + handler(unknownError) + return + } + + request.withNonisolatedUnsafeState { state in + state.setValue(firebaseToken, forHTTPHeaderField: "Authorization") + } + + guard let appCheck = self.appCheck else { + handler(nil) + return + } + + appCheck.getToken(forcingRefresh: false) { tokenResult in + if let error = tokenResult.error { + FirebaseLogger.log( + level: .debug, + service: "[FirebaseStorage]", + code: "I-STR000001", + message: "Failed to fetch AppCheck token. Error: \(error)" + ) + // Don't bubble the error up to the authorizer if we successfully + // got an auth token earlier. + } + + request.withNonisolatedUnsafeState { state in + state.setValue(token, forHTTPHeaderField: "X-Firebase-AppCheck") } + handler(nil) } } } @@ -120,7 +187,8 @@ final class StorageTokenAuthorizer: NSObject, GTMSessionFetcherAuthorizer, Senda return authHeader.hasPrefix("Firebase") } - let userEmail: String? + // Used for protocol conformance only. + let userEmail: String? = nil let callbackQueue: DispatchQueue private let googleAppID: String diff --git a/FirebaseStorage/Sources/Storage.swift b/FirebaseStorage/Sources/Storage.swift index a46c0fec9b4..bdb0937fe84 100644 --- a/FirebaseStorage/Sources/Storage.swift +++ b/FirebaseStorage/Sources/Storage.swift @@ -33,7 +33,7 @@ internal import FirebaseCoreExtension /// If you provide a custom instance of `FirebaseApp`, /// the storage location will be specified via the `FirebaseOptions.storageBucket` property. @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) -@objc(FIRStorage) open class Storage: NSObject { +@objc(FIRStorage) open class Storage: NSObject, @unchecked Sendable /* TODO: sendable */ { // MARK: - Public APIs /// The default `Storage` instance. diff --git a/FirebaseStorage/Sources/StorageReference.swift b/FirebaseStorage/Sources/StorageReference.swift index 4d3a9409360..0a3d0b705dd 100644 --- a/FirebaseStorage/Sources/StorageReference.swift +++ b/FirebaseStorage/Sources/StorageReference.swift @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +import FirebaseCoreInternal import Foundation /// `StorageReference` represents a reference to a Google Cloud Storage object. Developers can @@ -313,19 +314,17 @@ import Foundation /// under /// the current `StorageReference`. @objc(listAllWithCompletion:) - open func listAll(completion: @escaping ((_: StorageListResult?, _: Error?) -> Void)) { - var prefixes = [StorageReference]() - var items = [StorageReference]() + open func listAll(completion: @escaping @Sendable (_: StorageListResult?, _: Error?) -> Void) { + // Making this a closure changes its self-capture semantics, which causes + // a compiler error. + @Sendable func paginatedCompletion(_ listResult: StorageListResult?, _ error: Error?) { + var prefixes = [StorageReference]() + var items = [StorageReference]() - weak var weakSelf = self - - var paginatedCompletion: ((_: StorageListResult?, _: Error?) -> Void)? - paginatedCompletion = { (_ listResult: StorageListResult?, _ error: Error?) in if let error { completion(nil, error) return } - guard let strongSelf = weakSelf else { return } guard let listResult = listResult else { fatalError("internal error: both listResult and error are nil") } @@ -333,16 +332,13 @@ import Foundation items.append(contentsOf: listResult.items) if let pageToken = listResult.pageToken { - StorageListTask.listTask(reference: strongSelf, - queue: strongSelf.storage.dispatchQueue, + StorageListTask.listTask(reference: self, + queue: storage.dispatchQueue, pageSize: nil, previousPageToken: pageToken, completion: paginatedCompletion) } else { let result = StorageListResult(withPrefixes: prefixes, items: items, pageToken: nil) - - // Break the retain cycle we set up indirectly by passing the callback to `nextPage`. - paginatedCompletion = nil completion(result, nil) } } @@ -383,7 +379,7 @@ import Foundation /// prefixes under the current `StorageReference`. @objc(listWithMaxResults:completion:) open func list(maxResults: Int64, - completion: @escaping ((_: StorageListResult?, _: Error?) -> Void)) { + completion: @escaping (@Sendable (_: StorageListResult?, _: Error?) -> Void)) { if maxResults <= 0 || maxResults > 1000 { completion(nil, StorageError.invalidArgument( message: "Argument 'maxResults' must be between 1 and 1000 inclusive." @@ -416,7 +412,7 @@ import Foundation @objc(listWithMaxResults:pageToken:completion:) open func list(maxResults: Int64, pageToken: String, - completion: @escaping ((_: StorageListResult?, _: Error?) -> Void)) { + completion: @escaping (@Sendable (_: StorageListResult?, _: Error?) -> Void)) { if maxResults <= 0 || maxResults > 1000 { completion(nil, StorageError.invalidArgument( message: "Argument 'maxResults' must be between 1 and 1000 inclusive." @@ -484,8 +480,8 @@ import Foundation /// Deletes the object at the current path. /// - Parameter completion: A completion block which returns a nonnull error on failure. @objc(deleteWithCompletion:) - open func delete(completion: ((_: Error?) -> Void)?) { - let completionWrap = { (_: Data?, error: Error?) in + open func delete(completion: (@Sendable (_: Error?) -> Void)?) { + let completionWrap: @Sendable (Data?, Error?) -> Void = { (_: Data?, error: Error?) in if let completion { completion(error) } diff --git a/FirebaseStorage/Tests/Integration/StorageAsyncAwait.swift b/FirebaseStorage/Tests/Integration/StorageAsyncAwait.swift index fa780f24a6c..6cb0ddc64ba 100644 --- a/FirebaseStorage/Tests/Integration/StorageAsyncAwait.swift +++ b/FirebaseStorage/Tests/Integration/StorageAsyncAwait.swift @@ -18,7 +18,7 @@ import FirebaseStorage import XCTest @available(iOS 13.0, macOS 10.15, macCatalyst 13.0, tvOS 13.0, watchOS 6.0, *) -class StorageAsyncAwait: StorageIntegrationCommon { +class StorageAsyncAwait: StorageIntegrationCommon, @unchecked Sendable { func testGetMetadata() async throws { let ref = storage.reference().child("ios/public/1mb2") let result = try await ref.getMetadata() @@ -279,7 +279,7 @@ class StorageAsyncAwait: StorageIntegrationCommon { XCTAssertEqual(url.lastPathComponent, #function + "hello.txt") } - func testSimpleGetFile() throws { + @MainActor func testSimpleGetFile() throws { let expectation = self.expectation(description: #function) let ref = storage.reference(withPath: "ios/public/helloworld" + #function) let tmpDirURL = URL(fileURLWithPath: NSTemporaryDirectory()) @@ -427,7 +427,7 @@ class StorageAsyncAwait: StorageIntegrationCommon { XCTAssertNil(listResult.pageToken, "pageToken should be nil") } - private func waitForExpectations() { + @MainActor private func waitForExpectations() { let kTestTimeout = 60.0 waitForExpectations(timeout: kTestTimeout, handler: { error in diff --git a/FirebaseStorage/Tests/Integration/StorageIntegration.swift b/FirebaseStorage/Tests/Integration/StorageIntegration.swift index 2f1a59968e1..8afa1bd73af 100644 --- a/FirebaseStorage/Tests/Integration/StorageIntegration.swift +++ b/FirebaseStorage/Tests/Integration/StorageIntegration.swift @@ -14,6 +14,7 @@ import FirebaseAuth import FirebaseCore +import FirebaseCoreInternal @testable import FirebaseStorage import XCTest @@ -41,7 +42,7 @@ import XCTest } */ -class StorageResultTests: StorageIntegrationCommon { +@MainActor class StorageResultTests: StorageIntegrationCommon, @unchecked Sendable { func testGetMetadata() { let expectation = self.expectation(description: "testGetMetadata") let ref = storage.reference().child("ios/public/1mb") @@ -137,7 +138,7 @@ class StorageResultTests: StorageIntegrationCommon { waitForExpectations() } - func testNoDeadlocks() throws { + @MainActor func testNoDeadlocks() throws { let storage2 = Storage.storage(url: "") let expectation1 = expectation(description: #function) @@ -282,7 +283,7 @@ class StorageResultTests: StorageIntegrationCommon { "Failed to load file") let tmpDirURL = URL(fileURLWithPath: NSTemporaryDirectory()) let fileURL = tmpDirURL.appendingPathComponent("LargePutFile.txt") - var progressCount = 0 + let progressCount = FIRNonisolatedUnsafe(initialState: 0) try data.write(to: fileURL, options: .atomicWrite) @@ -290,7 +291,7 @@ class StorageResultTests: StorageIntegrationCommon { storage.uploadChunkSizeBytes = 256_000 let task = ref.putFile(from: fileURL) { result in - XCTAssertGreaterThanOrEqual(progressCount, 4) + XCTAssertGreaterThanOrEqual(progressCount.state, 4) self.assertResultSuccess(result) putFileExpectation.fulfill() } @@ -309,7 +310,9 @@ class StorageResultTests: StorageIntegrationCommon { XCTFail("Failed to get snapshot.progress") return } - progressCount = progressCount + 1 + progressCount.withNonisolatedUnsafeState { + $0 += 1 + } XCTAssertGreaterThanOrEqual(progress.completedUnitCount, uploadedBytes) uploadedBytes = progress.completedUnitCount } @@ -331,7 +334,7 @@ class StorageResultTests: StorageIntegrationCommon { "Failed to load file") let tmpDirURL = URL(fileURLWithPath: NSTemporaryDirectory()) let fileURL = tmpDirURL.appendingPathComponent("LargePutFile.txt") - var progressCount = 0 + let progressCount = FIRNonisolatedUnsafe(initialState: 0) try data.write(to: fileURL, options: .atomicWrite) @@ -340,8 +343,8 @@ class StorageResultTests: StorageIntegrationCommon { storage.uploadChunkSizeBytes = 1 let task = ref.putFile(from: fileURL) { result in - XCTAssertGreaterThanOrEqual(progressCount, 4) - XCTAssertLessThanOrEqual(progressCount, 6) + XCTAssertGreaterThanOrEqual(progressCount.state, 4) + XCTAssertLessThanOrEqual(progressCount.state, 6) self.assertResultSuccess(result) putFileExpectation.fulfill() } @@ -360,7 +363,9 @@ class StorageResultTests: StorageIntegrationCommon { XCTFail("Failed to get snapshot.progress") return } - progressCount = progressCount + 1 + progressCount.withNonisolatedUnsafeState { + $0 += 1 + } XCTAssertGreaterThanOrEqual(progress.completedUnitCount, uploadedBytes) uploadedBytes = progress.completedUnitCount } @@ -613,9 +618,9 @@ class StorageResultTests: StorageIntegrationCommon { waitForExpectations() } - private func assertMetadata(actualMetadata: StorageMetadata, - expectedContentType: String, - expectedCustomMetadata: [String: String]) { + private nonisolated func assertMetadata(actualMetadata: StorageMetadata, + expectedContentType: String, + expectedCustomMetadata: [String: String]) { XCTAssertEqual(actualMetadata.cacheControl, "cache-control") XCTAssertEqual(actualMetadata.contentDisposition, "content-disposition") XCTAssertEqual(actualMetadata.contentEncoding, "gzip") @@ -627,7 +632,7 @@ class StorageResultTests: StorageIntegrationCommon { } } - private func assertMetadataNil(actualMetadata: StorageMetadata) { + private nonisolated func assertMetadataNil(actualMetadata: StorageMetadata) { XCTAssertNil(actualMetadata.cacheControl) XCTAssertNil(actualMetadata.contentDisposition) XCTAssertEqual(actualMetadata.contentEncoding, "identity") @@ -827,7 +832,7 @@ class StorageResultTests: StorageIntegrationCommon { waitForExpectations() } - func testListAllFiles() { + @MainActor func testListAllFiles() { let expectation = self.expectation(description: #function) let ref = storage.reference(withPath: "ios/public/list") @@ -855,8 +860,9 @@ class StorageResultTests: StorageIntegrationCommon { }) } - private func assertResultSuccess(_ result: Result, - file: StaticString = #file, line: UInt = #line) { + private nonisolated func assertResultSuccess(_ result: Result, + file: StaticString = #filePath, + line: UInt = #line) { switch result { case let .success(value): XCTAssertNotNil(value, file: file, line: line) @@ -865,8 +871,9 @@ class StorageResultTests: StorageIntegrationCommon { } } - private func assertResultFailure(_ result: Result, - file: StaticString = #file, line: UInt = #line) { + private nonisolated func assertResultFailure(_ result: Result, + file: StaticString = #filePath, + line: UInt = #line) { switch result { case let .success(value): XCTFail("Unexpected success with value: \(value)") diff --git a/FirebaseStorage/Tests/Integration/StorageIntegrationCommon.swift b/FirebaseStorage/Tests/Integration/StorageIntegrationCommon.swift index 2cc09981167..5be6ff278cd 100644 --- a/FirebaseStorage/Tests/Integration/StorageIntegrationCommon.swift +++ b/FirebaseStorage/Tests/Integration/StorageIntegrationCommon.swift @@ -17,13 +17,13 @@ import FirebaseCore import FirebaseStorage import XCTest -class StorageIntegrationCommon: XCTestCase { +class StorageIntegrationCommon: XCTestCase, @unchecked Sendable { var app: FirebaseApp! var auth: Auth! var storage: Storage! - static var configured = false - static var once = false - static var signedIn = false + nonisolated(unsafe) static var configured = false + nonisolated(unsafe) static var once = false + nonisolated(unsafe) static var signedIn = false override class func setUp() { if !StorageIntegrationCommon.configured { @@ -32,14 +32,14 @@ class StorageIntegrationCommon: XCTestCase { } } - override func setUp() { - super.setUp() + override func setUp() async throws { + try await super.setUp() app = FirebaseApp.app() auth = Auth.auth(app: app) storage = Storage.storage(app: app!) if !StorageIntegrationCommon.signedIn { - signInAndWait() + await signInAndWait() } if !StorageIntegrationCommon.once { @@ -72,7 +72,7 @@ class StorageIntegrationCommon: XCTestCase { setupExpectation.fulfill() } } - waitForExpectations() + await waitForExpectations() } catch { XCTFail("Error thrown setting up files in setUp") } @@ -85,7 +85,7 @@ class StorageIntegrationCommon: XCTestCase { super.tearDown() } - private func signInAndWait() { + @MainActor private func signInAndWait() { let expectation = self.expectation(description: #function) auth.signIn(withEmail: Credentials.kUserName, password: Credentials.kPassword) { result, error in @@ -97,7 +97,7 @@ class StorageIntegrationCommon: XCTestCase { waitForExpectations() } - private func waitForExpectations() { + @MainActor private func waitForExpectations() { let kTestTimeout = 60.0 waitForExpectations(timeout: kTestTimeout, handler: { error in @@ -108,7 +108,7 @@ class StorageIntegrationCommon: XCTestCase { } private func assertResultSuccess(_ result: Result, - file: StaticString = #file, line: UInt = #line) { + file: StaticString = #filePath, line: UInt = #line) { switch result { case let .success(value): XCTAssertNotNil(value, file: file, line: line)