-
Notifications
You must be signed in to change notification settings - Fork 52
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
[Paywalls V2] Handles configuring while the device is locked #2077
base: main
Are you sure you want to change the base?
Conversation
private val mockApplication = mockk<Application>(relaxed = true).apply { | ||
every { applicationContext } returns this | ||
} | ||
protected val mockContext = mockk<Context>(relaxed = true).apply { | ||
every { | ||
applicationContext | ||
} returns mockApplication | ||
protected val mockContext = InstrumentationRegistry.getInstrumentation().targetContext | ||
private val mockApplication = (mockContext.applicationContext as Application).apply { | ||
shadowOf(this).grantPermissions(Manifest.permission.INTERNET) |
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 is required, because a mocked Context
doesn't return anything for getSystemService()
, which is now used to get the UserManager
and check whether the device is locked.
📸 Snapshot Test179 unchanged
🛸 Powered by Emerge Tools |
val prefs = PreferenceManager.getDefaultSharedPreferences(application) | ||
val isUserUnlocked = UserManagerCompat.isUserUnlocked(context) | ||
val prefs = try { | ||
PreferenceManager.getDefaultSharedPreferences(if (isUserUnlocked) application else context) |
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 have only one concern with this PR... That is, if the shared preferences we obtain with the given context is different than the one we obtain through the application, there is a chance of us losing the entire cached shared preferences right? This could result in using anonymous ids inadvertently (unless the developer always passes us the user id when configuring), losing entitlements until the cache can be refreshed, or losing subscriber attributes.
I'm just wondering if we should try to store this data in files, instead of shared preferences to avoid these possible issues...
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.
Moving this back to draft, as this is indeed an issue. Good one! I'm not entirely sure if moving the data in a non-sharedprefs file is a solution, as I believe the data directory is different for a "regular" context vs a "device-protected-storage" context. So we'd still end up with 2 copies, depending on when the SDK is configured.
Thinking about this, I'm starting to think we should not try to be smart about it, and just pass the Context
directly to getDefaultSharedPreferences()
. That way app developers that need this can make sure to always give us a consistent one (e.g. always a "device-protected-storage" context).
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2077 +/- ##
==========================================
- Coverage 81.24% 81.18% -0.07%
==========================================
Files 267 267
Lines 8867 8875 +8
Branches 1259 1261 +2
==========================================
+ Hits 7204 7205 +1
- Misses 1144 1150 +6
- Partials 519 520 +1 ☔ View full report in Codecov by Sentry. |
Description
As the title says. The strategy is as follows:
Context
directly to getSharedPreferences
, instead of theApplication
. This is done so developers can pass a specially-preparedContext
usingcreateDeviceProtectedStorageContext()
. This is required by Android to getSharedPreferences
while the device is locked.SharedPreferences
, we check if the device is locked. If so, we provide further instructions in the error.Note: labeled this as a fix, let me know what you think!