Skip to content

Entitlement verification: Invalidate DeviceCache and ETag cache if verification mode enabled and cached value is NOT_REQUESTED#844

Merged
tonidero merged 6 commits into
entitlements-verificationfrom
toniricodiez/sdk-2896-invalidate-devicecache-cache-if
Mar 8, 2023
Merged

Entitlement verification: Invalidate DeviceCache and ETag cache if verification mode enabled and cached value is NOT_REQUESTED#844
tonidero merged 6 commits into
entitlements-verificationfrom
toniricodiez/sdk-2896-invalidate-devicecache-cache-if

Conversation

@tonidero

@tonidero tonidero commented Mar 6, 2023

Copy link
Copy Markdown
Contributor

Description

Completes SDK-2896.

This PR will invalidate the CustomerInfo from the DeviceCache and the entire eTag cache if verification is enabled but the cached CustomerInfo verification result is NOT_REQUESTED.

@tonidero tonidero added the pr:feat A new feature label Mar 6, 2023
Comment thread common/src/main/java/com/revenuecat/purchases/common/networking/ETagManager.kt Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about this for a while, and I think invalidating it on first configure should cover all cases, since when we logIn, we need a new CustomerInfo and we also delete the old cached CustomerInfo for the previous user.

I was also thinking about where this logic belongs to, and I thought that it fit well within the IdentityManager. Lmk if you had other ideas though!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, all combined in a single place 👍🏻

@tonidero tonidero marked this pull request as ready for review March 6, 2023 17:07
@tonidero tonidero requested a review from a team March 6, 2023 17:07
@codecov

codecov Bot commented Mar 6, 2023

Copy link
Copy Markdown

Codecov Report

Merging #844 (3410334) into entitlements-verification (86d1287) will increase coverage by 0.04%.
The diff coverage is 90.32%.

❗ Current head 3410334 differs from pull request most recent head 16f47b9. Consider uploading reports for the commit 16f47b9 to get more accurate results

@@                      Coverage Diff                      @@
##           entitlements-verification     #844      +/-   ##
=============================================================
+ Coverage                      82.62%   82.66%   +0.04%     
=============================================================
  Files                            137      137              
  Lines                           4501     4512      +11     
  Branches                         587      589       +2     
=============================================================
+ Hits                            3719     3730      +11     
  Misses                           562      562              
  Partials                         220      220              
Impacted Files Coverage Δ
...revenuecat/purchases/common/CustomerInfoFactory.kt 90.00% <0.00%> (ø)
...n/java/com/revenuecat/purchases/EntitlementInfo.kt 48.35% <0.00%> (ø)
.../java/com/revenuecat/purchases/EntitlementInfos.kt 80.00% <0.00%> (ø)
...om/revenuecat/purchases/strings/IdentityStrings.kt 0.00% <ø> (ø)
...java/com/revenuecat/purchases/common/HTTPClient.kt 88.63% <100.00%> (-0.17%) ⬇️
...revenuecat/purchases/common/caching/DeviceCache.kt 95.08% <100.00%> (ø)
...enuecat/purchases/common/networking/ETagManager.kt 97.95% <100.00%> (ø)
...venuecat/purchases/common/networking/HTTPResult.kt 100.00% <100.00%> (ø)
...at/purchases/common/verification/SigningManager.kt 100.00% <100.00%> (ø)
...m/revenuecat/purchases/identity/IdentityManager.kt 97.26% <100.00%> (+0.48%) ⬆️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tonidero tonidero force-pushed the entitlement-verification-cache-verification-errors branch from a88f8d6 to 2c0b25f Compare March 7, 2023 17:29
Base automatically changed from entitlement-verification-cache-verification-errors to entitlements-verification March 7, 2023 17:29
Comment thread strings/src/main/java/com/revenuecat/purchases/strings/IdentityStrings.kt Outdated
Comment on lines 24 to 25

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm doing this in iOS to validate if this abstraction makes sense (putting this in IdentityManager) have an improvement suggestion: you can call backend.clearCaches instead of injecting ETagManager here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And maybe instead of injecting the verification mode, we could expose it from Backend?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestions! I liked them a lot so applied in fdb918a

Comment on lines 137 to 138

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious about the difference here, why one is CamelCase and the other uppercase?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VerificationResult is an enum and the convention is to use uppercases (though there is some discussion around that). While SignatureVerificationMode is a sealed class, so types of that sealed class are basically classes and should use CamelCase

@tonidero tonidero force-pushed the toniricodiez/sdk-2896-invalidate-devicecache-cache-if branch from 4b42115 to fdb918a Compare March 8, 2023 11:55
@tonidero tonidero requested review from NachoSoto and vegaro March 8, 2023 11:58
@tonidero tonidero merged commit 6cebf81 into entitlements-verification Mar 8, 2023
@tonidero tonidero deleted the toniricodiez/sdk-2896-invalidate-devicecache-cache-if branch March 8, 2023 15:25
NachoSoto added a commit to RevenueCat/purchases-ios that referenced this pull request Mar 13, 2023
…ion is enabled but cached `CustomerInfo` is not

Android counterpart: RevenueCat/purchases-android#844
NachoSoto added a commit to RevenueCat/purchases-ios that referenced this pull request Mar 13, 2023
…ion is enabled but cached `CustomerInfo` is not (#2330)

Android counterpart:
RevenueCat/purchases-android#844
tonidero added a commit that referenced this pull request Mar 15, 2023
…rification mode enabled and cached value is NOT_REQUESTED (#844)

### Description
Completes
[SDK-2896](https://linear.app/revenuecat/issue/SDK-2896/invalidate-devicecache-cache-if-entitlement-verification-is-missing).

This PR will invalidate the `CustomerInfo` from the `DeviceCache` and
the entire eTag cache if verification is enabled but the cached
`CustomerInfo` verification result is `NOT_REQUESTED`.
@tonidero tonidero mentioned this pull request Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:feat A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants