From 5dd57f9de42c02d6a94f3af4d8cf3d9b81ec6661 Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Mon, 9 Dec 2024 19:13:15 +0100 Subject: [PATCH] Always notify Account when FirestoreAccountStorage receives snapshot (#49) # Always notify Account when FirestoreAccountStorage receives snapshot ## :recycle: Current situation & Problem With https://github.com/StanfordSpezi/SpeziAccount/pull/79 we only forward "complete" account details to the `setupComplete` closure of the `AccountSetup` view. This requires that the StorageProvider always updates the account details (to clear the incomplete flag) even if there aren't any keys stored in the storage provider. This wasn't the case for FirestoreAccountStorage resulting in the `setupComplete` closure to never be called and the `incomplete` flag to never be cleared. ## :gear: Release Notes * Fixed an issue where the `incomplete` flag would never be cleared if there weren't any details stored for an account. ## :books: Documentation -- ## :white_check_mark: Testing -- ## :pencil: Code of Conduct & Contributing Guidelines By submitting creating this pull request, you agree to follow our [Code of Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md): - [x] I agree to follow the [Code of Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md). --- .github/workflows/build-and-test.yml | 2 +- Package.swift | 2 +- Sources/SpeziFirebaseAccount/FirebaseAccountService.swift | 5 ++++- .../FirestoreAccountStorage.swift | 4 ---- Sources/SpeziFirestore/DocumentReference+AsyncAwait.swift | 6 +++--- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 15307e3..ab2e53f 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -32,7 +32,7 @@ jobs: setupfirebaseemulator: true path: Tests/UITests customcommand: | - firebase emulators:exec 'set -o pipefail && xcodebuild test -project UITests.xcodeproj -scheme TestApp -destination "platform=iOS Simulator,name=iPhone 15 Pro" -resultBundlePath UITests.xcresult -derivedDataPath ".derivedData" CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO -skipPackagePluginValidation -skipMacroValidation | xcbeautify' + firebase emulators:exec 'set -o pipefail && xcodebuild test -project UITests.xcodeproj -scheme TestApp -destination "platform=iOS Simulator,name=iPhone 16 Pro" -resultBundlePath UITests.xcresult -derivedDataPath ".derivedData" CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO -skipPackagePluginValidation -skipMacroValidation | xcbeautify' uploadcoveragereport: name: Upload Coverage Report needs: [buildandtest, buildandtestuitests] diff --git a/Package.swift b/Package.swift index e49a1d2..1ee51c9 100644 --- a/Package.swift +++ b/Package.swift @@ -29,7 +29,7 @@ let package = Package( .package(url: "https://github.com/StanfordSpezi/SpeziFoundation", from: "2.0.0"), .package(url: "https://github.com/StanfordSpezi/Spezi", from: "1.7.1"), .package(url: "https://github.com/StanfordSpezi/SpeziViews", from: "1.6.0"), - .package(url: "https://github.com/StanfordSpezi/SpeziAccount", from: "2.0.0"), + .package(url: "https://github.com/StanfordSpezi/SpeziAccount", from: "2.1.1"), .package(url: "https://github.com/firebase/firebase-ios-sdk", from: "11.0.0"), .package(url: "https://github.com/apple/swift-atomics.git", from: "1.2.0") ] + swiftLintPackage(), diff --git a/Sources/SpeziFirebaseAccount/FirebaseAccountService.swift b/Sources/SpeziFirebaseAccount/FirebaseAccountService.swift index af19bfa..798d21c 100644 --- a/Sources/SpeziFirebaseAccount/FirebaseAccountService.swift +++ b/Sources/SpeziFirebaseAccount/FirebaseAccountService.swift @@ -659,7 +659,10 @@ extension FirebaseAccountService { actionSemaphore.signal() } - let details = buildUser(user, isNewUser: false, mergeWith: details) + // make sure to keep the `newUser` flag + let consideredNewUser = account.details?.isNewUser ?? false + + let details = buildUser(user, isNewUser: consideredNewUser, mergeWith: details) logger.debug("Update user details due to updates in the externally stored account details.") account.supplyUserDetails(details) } diff --git a/Sources/SpeziFirebaseAccountStorage/FirestoreAccountStorage.swift b/Sources/SpeziFirebaseAccountStorage/FirestoreAccountStorage.swift index 85ffbbe..533a8c0 100644 --- a/Sources/SpeziFirebaseAccountStorage/FirestoreAccountStorage.swift +++ b/Sources/SpeziFirebaseAccountStorage/FirestoreAccountStorage.swift @@ -163,10 +163,6 @@ public actor FirestoreAccountStorage: AccountStorageProvider { let details = buildAccountDetails(from: snapshot, keys: Array(keys)) - guard !details.isEmpty else { - return - } - let localCache = localCache await localCache.communicateRemoteChanges(for: accountId, details) diff --git a/Sources/SpeziFirestore/DocumentReference+AsyncAwait.swift b/Sources/SpeziFirestore/DocumentReference+AsyncAwait.swift index d15c381..5467789 100644 --- a/Sources/SpeziFirestore/DocumentReference+AsyncAwait.swift +++ b/Sources/SpeziFirestore/DocumentReference+AsyncAwait.swift @@ -80,7 +80,7 @@ extension DocumentReference { /// - isolation: The actor isolation to inherit. /// - value: An instance of `Encodable` to be encoded to a document. /// - encoder: An encoder instance to use to run the encoding. - public func setData( // swiftlint:disable:this function_default_parameter_at_end + public func setData( isolation: isolated (any Actor)? = #isolation, from value: T, encoder: FirebaseFirestore.Firestore.Encoder = FirebaseFirestore.Firestore.Encoder() @@ -110,7 +110,7 @@ extension DocumentReference { /// - merge: Whether to merge the provided `Encodable` into any existing /// document. /// - encoder: An encoder instance to use to run the encoding. - public func setData( // swiftlint:disable:this function_default_parameter_at_end + public func setData( isolation: isolated (any Actor)? = #isolation, from value: T, merge: Bool, @@ -145,7 +145,7 @@ extension DocumentReference { /// merge. Fields can contain dots to reference nested fields within the /// document. /// - encoder: An encoder instance to use to run the encoding. - public func setData( // swiftlint:disable:this function_default_parameter_at_end + public func setData( isolation: isolated (any Actor)? = #isolation, from value: T, mergeFields: [Any],