Skip to content
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.

Sign up for GitHub

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

[Paywalls V2] Handles configuring while the device is locked #2077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f6e7c3
Uses the provided context instead of the application when the device …
JayShortway Jan 23, 2025
cd21438
Fixes PurchasesConfigureTest.
JayShortway Jan 23, 2025
8717459
Refactors PurchasesFactoryTest to use shadows.
JayShortway Jan 23, 2025
efaaf1f
Revert "Refactors PurchasesFactoryTest to use shadows."
JayShortway Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import android.content.pm.PackageManager
import android.preference.PreferenceManager
import androidx.annotation.VisibleForTesting
import androidx.core.os.UserManagerCompat
import com.revenuecat.purchases.common.AppConfig
import com.revenuecat.purchases.common.Backend
import com.revenuecat.purchases.common.BackendHelper
Expand Down Expand Up @@ -56,7 +57,7 @@
private val apiKeyValidator: APIKeyValidator = APIKeyValidator(),
) {

@Suppress("LongMethod", "LongParameterList")
@Suppress("LongMethod", "LongParameterList", "CyclomaticComplexMethod")
fun createPurchases(
configuration: PurchasesConfiguration,
platformInfo: PlatformInfo,
Expand Down Expand Up @@ -84,7 +85,24 @@
forceSigningError,
)

val prefs = PreferenceManager.getDefaultSharedPreferences(application)
val isUserUnlocked = UserManagerCompat.isUserUnlocked(context)
val prefs = try {
PreferenceManager.getDefaultSharedPreferences(if (isUserUnlocked) application else context)
Copy link
Contributor

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...

Copy link
Member Author

@JayShortway JayShortway Jan 23, 2025 •
edited
Loading

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).

} catch (e: IllegalStateException) {

Check warning on line 91 in purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt

View check run for this annotation

Codecov / codecov/patch

purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt#L91 

Added line #L91 was not covered by tests
@Suppress("MaxLineLength")
if (!isUserUnlocked) {
throw IllegalStateException(
"Trying to configure Purchases while the device is locked. If you need to support this " +

Check warning on line 95 in purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt

View check run for this annotation

Codecov / codecov/patch

purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt#L94-L95 

Added lines #L94 - L95 were not covered by tests
"scenario, ensure you provide a Context created with " +
"`createDeviceProtectedStorageContext()`.\nSee " +
"https://developer.android.com/reference/android/content/Context#createDeviceProtectedStorageContext() " +
"for more info.",
e,

Check warning on line 100 in purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt

View check run for this annotation

Codecov / codecov/patch

purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt#L100 

Added line #L100 was not covered by tests
)
} else {
throw e

Check warning on line 103 in purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt

View check run for this annotation

Codecov / codecov/patch

purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt#L103 

Added line #L103 was not covered by tests
}
}

val eTagManager = ETagManager(context)

Expand Down
14 changes: 6 additions & 8 deletions purchases/src/test/java/com/revenuecat/purchases/BasePurchasesTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@

package com.revenuecat.purchases

import android.Manifest
import android.app.Activity
import android.app.Application
import android.content.Context
import androidx.lifecycle.ProcessLifecycleOwner
import androidx.test.platform.app.InstrumentationRegistry
import com.android.billingclient.api.Purchase
import com.android.billingclient.api.PurchaseHistoryRecord
import com.revenuecat.purchases.PurchasesAreCompletedBy.REVENUECAT
Expand Down Expand Up @@ -50,19 +51,16 @@ import io.mockk.slot
import io.mockk.unmockkStatic
import org.junit.After
import org.junit.Before
import org.robolectric.Shadows.shadowOf

internal open class BasePurchasesTest {
protected val mockBillingAbstract: BillingAbstract = mockk()
protected val mockBackend: Backend = mockk()
protected val mockCache: DeviceCache = mockk()
protected val updatedCustomerInfoListener: UpdatedCustomerInfoListener = mockk()
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)
Comment on lines -59 to +63
Copy link
Member Author

@JayShortway JayShortway Jan 23, 2025 •
edited
Loading

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.

}
protected val mockIdentityManager = mockk<IdentityManager>()
protected val mockSubscriberAttributesManager = mockk<SubscriberAttributesManager>()
Expand Down