-
Notifications
You must be signed in to change notification settings - Fork 82
Use Block store
to backup anonymous user ids across installations
#2595
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
Use Block store
to backup anonymous user ids across installations
#2595
Conversation
…s users purchases
@@ -6,7 +6,7 @@ | |||
|
|||
<application | |||
android:name="com.revenuecat.purchasetester.MainApplication" | |||
android:allowBackup="true" | |||
android:allowBackup="false" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This disables auto backup, which helps to test block store. Auto backup should also help minimize the spread of this issue in any case, and shouldn't conflict with these changes.
const val BLOCKSTORE_MAX_ENTRIES = 16 | ||
} | ||
|
||
fun storeUserIdIfNeeded(customerInfo: CustomerInfo) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are only storing if:
- The user id is anonymous
- Has purchases
- No previous user id was stored
- We haven't reached the maximum 16 entries of storage for Block store.
We try to store:
- When obtaining CustomerInfo on app foreground
- When making a purchase
} | ||
} | ||
|
||
fun recoverAndAliasBlockstoreUserIfNeeded(callback: () -> Unit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will try to recover the user Id from Block store and alias the users together if:
- Current user is anonymous
- There is a user Id cached in Block store
- That user id is not the same as the current user id.
We try to do this:
- When calling
restorePurchases
- When completing a purchase
} | ||
} | ||
|
||
fun clearBlockstoreUserIdBackupIfNeeded(callback: () -> Unit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We clear the block store backup when a user logs in. In that case, we shouldn't rely on this system anymore and the developer should just rely on their user ids.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we not alias whatever ID we have in the block store with the new ID? We could have an ID in the Block store with consumed purchases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the moment the user calls logIn
, those users already get aliased (as long as the new user id is not anonymous) so we don't need to alias them again. The developer can just rely in the user Id they passed us, which should already have the purchases of the user before logging in.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2595 +/- ##
==========================================
+ Coverage 78.32% 78.38% +0.06%
==========================================
Files 300 301 +1
Lines 11126 11229 +103
Branches 1551 1561 +10
==========================================
+ Hits 8714 8802 +88
- Misses 1732 1741 +9
- Partials 680 686 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of this!! 💪
dispatch { callback?.onError(error) } | ||
}, | ||
) | ||
blockstoreHelper.clearBlockstoreUserIdBackupIfNeeded { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if people do things very wrong and call logIn
with an anonymous ID? Should we maybe check for that (to be absolutely sure), and alias in that case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say they are seriously holding it wrong 😅 And in that case, this would clear the block store user id, and it would be cached again on the next app foreground/customer info refresh.
So I guess the case we would miss in this case is if you make a purchase before the logIn, then logIn, then uninstall before the new userId is cached. We would lose the userId that did the purchase.
All things said, I think it's quite an edge case... but then again, I think we should be able to handle it yeah... I will try to do this today!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah wait, logIn didn't create aliases, if the new user id is anonymous I forgot... So yeah, we might need to alias the users ourselves in this case. Still quite an edge case, but better to control it yeah
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, what I'm going to do is to avoid clearing the user id from block store if the new user id is anonymous. It won't give the new user any entitlements, but that's in line with what was happening now I think. If the new anonymous user restores, they should still be able to obtain those purchases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this could still cause some issues, if they logIn with a non-anonymous user AFTER logging in with an anonymous user... TBH, not sure how much we want to handle this edge case though... Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avoid clearing the user id from block store if the new user id is anonymous
I think that makes sense!
I guess this could still cause some issues, if they logIn with a non-anonymous user AFTER logging in with an anonymous user...
A restore would recover from this I guess? If so, I'd say that's acceptable.
purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesOrchestrator.kt
Show resolved
Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/blockstore/BlockstoreHelper.kt
Outdated
Show resolved
Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/blockstore/BlockstoreHelper.kt
Outdated
Show resolved
Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/blockstore/BlockstoreHelper.kt
Outdated
Show resolved
Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/blockstore/BlockstoreHelper.kt
Outdated
Show resolved
Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/blockstore/BlockstoreHelper.kt
Outdated
Show resolved
Hide resolved
} | ||
} | ||
|
||
fun clearBlockstoreUserIdBackupIfNeeded(callback: () -> Unit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we not alias whatever ID we have in the block store with the new ID? We could have an ID in the Block store with consumed purchases?
purchases/src/main/kotlin/com/revenuecat/purchases/identity/IdentityManager.kt
Outdated
Show resolved
Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/strings/IdentityStrings.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the changes! 🙏 Just the configure vs onAppForegrounded thing, but not a blocker for me!
purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesOrchestrator.kt
Show resolved
Hide resolved
…dd-blockstore-anonymous-user-id-caching-and-recovery
purchases/src/main/kotlin/com/revenuecat/purchases/blockstore/BlockstoreHelper.kt
Outdated
Show resolved
Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/blockstore/BlockstoreHelper.kt
Outdated
Show resolved
Hide resolved
…nd we try to clear the cached value before that happens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this!!
applicationContext: Context, | ||
private val identityManager: IdentityManager, | ||
private val blockstoreClient: BlockstoreClient = Blockstore.getClient(applicationContext), | ||
private val ioScope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.IO.limitedParallelism(1)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the limited parallelism to avoid concurrency issues? If so, I guess we could still technically have 2 instances of BlockStoreHelper
. Not a huge problem probably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should only be able to have one right? Since it's built once and attached to the purchases orchestrator if I'm correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And yeah, this is to solve a possible race condition of a user logging in after we start storing the user id in blockstore, and it executing the clearing of blockstore before actually storing the user id, ending up with a user ID stored that has a non-anonymous alias. It's very unlikely but I thought if we execute these operations sequentially, this shouldn't be an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea true! But there's nothing enforcing this to be a singleton (we could make the mistake of constructing BlockStoreHelper()
in some other place), but I think that's totally fine.
purchases/src/main/kotlin/com/revenuecat/purchases/blockstore/BlockstoreHelper.kt
Show resolved
Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/blockstore/BlockstoreHelper.kt
Outdated
Show resolved
Hide resolved
**This is an automatic release.** ## RevenueCat SDK ### 🐞 Bugfixes * Use `Block store` to backup anonymous user ids across installations (#2595) via Toni Rico (@tonidero) ## RevenueCatUI SDK ### Paywallv2 #### 🐞 Bugfixes * Fixes price formatting discrepancies on Paywalls for `{{ product.price_per_[day|week|month|year] }}` (#2604) via JayShortway (@JayShortway) ### 🔄 Other Changes * Revert dokka 2 and gradle 9 update (#2618) via Toni Rico (@tonidero) * Introduce runtime annotations library and add stability annotations for increasing UI performances (#2608) via Jaewoong Eum (@skydoves) * Override presented offering context paywalls without offering (#2612) via Toni Rico (@tonidero) * Add APIs for hybrid SDKs to set presentedOfferingContext (#2610) via Toni Rico (@tonidero) * Bump Baseline Profiles to 1.4.0 and update profiles (#2611) via Jaewoong Eum (@skydoves) * Migrate deprecated kotlinOptions to compilerOptions (#2607) via Jaewoong Eum (@skydoves) * [AUTOMATIC][Paywalls V2] Updates paywall-preview-resources submodule (#2613) via RevenueCat Git Bot (@RCGitBot) * Migrate amazon & debugview modules to KTS (#2327) via Jaewoong Eum (@skydoves) * Update to Dokka 2.0.0 (#2609) via Toni Rico (@tonidero) * Add log when restoring purchases finds no purchases with some troubleshooting (#2599) via Toni Rico (@tonidero) Co-authored-by: revenuecat-ops <ops@revenuecat.com>
…2595) ### Description Based on #2594 This uses [Block store](https://developer.android.com/identity/block-store) to store the anonymous user id in the cloud when we find it has any purchases. This user ID will be recovered and aliased together with any new anonymous user id when restoring or completing a purchase. This will be useful to avoid issues when trying to restore purchases with consumed consumables that were meant to never be consumed.
Description
Based on #2594
This uses Block store to store the anonymous user id in the cloud when we find it has any purchases. This user ID will be recovered and aliased together with any new anonymous user id when restoring or completing a purchase.
This will be useful to avoid issues when trying to restore purchases with consumed consumables that were meant to never be consumed.