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.

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

Avoid recreating purchases same config #1866

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ class Purchases internal constructor(
@Synchronized get() =
if (purchasesOrchestrator.finishTransactions) {
PurchasesAreCompletedBy.REVENUECAT
} else PurchasesAreCompletedBy.MY_APP
} else {
PurchasesAreCompletedBy.MY_APP
}

@Synchronized set(value) {
purchasesOrchestrator.finishTransactions = when (value) {
Expand Down Expand Up @@ -882,7 +884,12 @@ class Purchases internal constructor(
configuration: PurchasesConfiguration,
): Purchases {
if (isConfigured) {
infoLog(ConfigureStrings.INSTANCE_ALREADY_EXISTS)
if (backingFieldSharedInstance?.purchasesOrchestrator?.configuration == configuration) {
infoLog(ConfigureStrings.INSTANCE_ALREADY_EXISTS_WITH_SAME_CONFIG)
return sharedInstance
} else {
infoLog(ConfigureStrings.INSTANCE_ALREADY_EXISTS)
}
}
return PurchasesFactory().createPurchases(
configuration,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ open class PurchasesConfiguration(builder: Builder) {
val pendingTransactionsForPrepaidPlansEnabled: Boolean

init {
this.context = builder.context
this.context = builder.context.applicationContext
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed we could potentially be leaking an activity if the developer gave us the activity context and we hold a reference to this object. So I changed it here so we only hold the application context, which is the one developers should normally give us.

this.apiKey = builder.apiKey.trim()
this.appUserID = builder.appUserID
this.purchasesAreCompletedBy = builder.purchasesAreCompletedBy
Expand Down Expand Up @@ -117,7 +117,9 @@ open class PurchasesConfiguration(builder: Builder) {
purchasesAreCompletedBy(
if (observerMode) {
PurchasesAreCompletedBy.MY_APP
} else PurchasesAreCompletedBy.REVENUECAT,
} else {
PurchasesAreCompletedBy.REVENUECAT
},
)
}

Expand Down Expand Up @@ -238,4 +240,40 @@ open class PurchasesConfiguration(builder: Builder) {
return PurchasesConfiguration(this)
}
}

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false

other as PurchasesConfiguration

if (context != other.context) return false
if (apiKey != other.apiKey) return false
if (appUserID != other.appUserID) return false
if (purchasesAreCompletedBy != other.purchasesAreCompletedBy) return false
if (showInAppMessagesAutomatically != other.showInAppMessagesAutomatically) return false
if (service != other.service) return false
if (store != other.store) return false
if (diagnosticsEnabled != other.diagnosticsEnabled) return false
if (dangerousSettings != other.dangerousSettings) return false
if (verificationMode != other.verificationMode) return false
if (pendingTransactionsForPrepaidPlansEnabled != other.pendingTransactionsForPrepaidPlansEnabled) return false

return true
}

override fun hashCode(): Int {
var result = context.hashCode()
result = 31 * result + apiKey.hashCode()
result = 31 * result + (appUserID?.hashCode() ?: 0)
result = 31 * result + purchasesAreCompletedBy.hashCode()
result = 31 * result + showInAppMessagesAutomatically.hashCode()
result = 31 * result + (service?.hashCode() ?: 0)
result = 31 * result + store.hashCode()
result = 31 * result + diagnosticsEnabled.hashCode()
result = 31 * result + dangerousSettings.hashCode()
result = 31 * result + verificationMode.hashCode()
result = 31 * result + pendingTransactionsForPrepaidPlansEnabled.hashCode()
return result
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ internal class PurchasesFactory(
paywallPresentedCache,
purchasesStateProvider,
dispatcher = dispatcher,
configuration = configuration,
)

return Purchases(purchasesOrchestrator)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ internal class PurchasesOrchestrator(
// This is nullable due to: https://github.com/RevenueCat/purchases-flutter/issues/408
private val mainHandler: Handler? = Handler(Looper.getMainLooper()),
private val dispatcher: Dispatcher,
internal val configuration: PurchasesConfiguration,
) : LifecycleDelegate, CustomActivityLifecycleHandler {

internal var state: PurchasesState
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ internal object ConfigureStrings {
"purchase."
const val INSTANCE_ALREADY_EXISTS = "Purchases instance already set. " +
"Did you mean to configure two Purchases objects?"
const val INSTANCE_ALREADY_EXISTS_WITH_SAME_CONFIG = "Purchases instance already set with the same " +
"configuration. Ignoring duplicate call."
}
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ internal open class BasePurchasesTest {
paywallPresentedCache = paywallPresentedCache,
purchasesStateCache = purchasesStateProvider,
dispatcher = SyncDispatcher(),
configuration = PurchasesConfiguration.Builder(mockContext, "api_key").build()
)
purchases = Purchases(purchasesOrchestrator)
Purchases.sharedInstance = purchases
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@

package com.revenuecat.purchases

import android.content.Context
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.revenuecat.purchases.PurchasesAreCompletedBy.MY_APP
import com.revenuecat.purchases.PurchasesAreCompletedBy.REVENUECAT
import com.revenuecat.purchases.common.PlatformInfo
import io.mockk.mockk
import org.assertj.core.api.Assertions.assertThat
import org.junit.Test
import org.junit.runner.RunWith
Expand Down Expand Up @@ -69,4 +71,100 @@ internal class PurchasesConfigureTest : BasePurchasesTest() {
Purchases.configure(builder.build())
assertThat(Purchases.sharedInstance.store).isEqualTo(Store.PLAY_STORE)
}

@Test
fun `Calling configure multiple times with same configuration does not create a new instance`() {
val builder = PurchasesConfiguration.Builder(mockContext, "api_key")
val instance1 = Purchases.configure(builder.build())
assertThat(Purchases.sharedInstance).isEqualTo(instance1)
val instance2 = Purchases.configure(builder.build())
assertThat(Purchases.sharedInstance).isEqualTo(instance2)
assertThat(instance2).isEqualTo(instance1)
}

@Test
fun `Calling configure multiple times with different configuration does create a new instance`() {
val config1 = PurchasesConfiguration.Builder(mockContext, "api_key").build()
val instance1 = Purchases.configure(config1)
assertThat(Purchases.sharedInstance).isEqualTo(instance1)
val config2 = PurchasesConfiguration.Builder(mockContext, "api_key2").build()
val instance2 = Purchases.configure(config2)
assertThat(Purchases.sharedInstance).isEqualTo(instance2)
assertThat(instance2).isNotEqualTo(instance1)
}

@Test
fun `configurations with same properties are equal`() {
val config1 = PurchasesConfiguration.Builder(mockContext, "api_key").build()
val config2 = PurchasesConfiguration.Builder(mockContext, "api_key").build()

assertThat(config1).isEqualTo(config2)
assertThat(config1.hashCode()).isEqualTo(config2.hashCode())
}

@Test
fun `configurations with different api keys are not equal`() {
val config1 = PurchasesConfiguration.Builder(mockContext, "api_key1").build()
val config2 = PurchasesConfiguration.Builder(mockContext, "api_key2").build()

assertThat(config1).isNotEqualTo(config2)
}

@Test
fun `configurations with different app user IDs are not equal`() {
val config1 = PurchasesConfiguration.Builder(mockContext, "api_key").appUserID("user1").build()
val config2 = PurchasesConfiguration.Builder(mockContext, "api_key").appUserID("user2").build()

assertThat(config1).isNotEqualTo(config2)
}

@Test
fun `configurations with different purchasesAreCompletedBy are not equal`() {
val config1 = PurchasesConfiguration.Builder(mockContext, "api_key").purchasesAreCompletedBy(MY_APP).build()
val config2 = PurchasesConfiguration.Builder(mockContext, "api_key").purchasesAreCompletedBy(REVENUECAT).build()

assertThat(config1).isNotEqualTo(config2)
}

@Test
fun `configurations with different stores are not equal`() {
val config1 = PurchasesConfiguration.Builder(mockContext, "api_key").store(Store.PLAY_STORE).build()
val config2 = PurchasesConfiguration.Builder(mockContext, "api_key").store(Store.AMAZON).build()

assertThat(config1).isNotEqualTo(config2)
}

@Test
fun `configurations with different contexts are not equal`() {
val context1 = mockContext
val context2 = mockk<Context>()
val config1 = PurchasesConfiguration.Builder(context1, "api_key").build()
val config2 = PurchasesConfiguration.Builder(context2, "api_key").build()

assertThat(config1).isNotEqualTo(config2)
}

@Test
fun `configurations with different verificationMode are not equal`() {
val config1 = PurchasesConfiguration.Builder(mockContext, "api_key")
.entitlementVerificationMode(EntitlementVerificationMode.DISABLED)
.build()
val config2 = PurchasesConfiguration.Builder(mockContext, "api_key")
.entitlementVerificationMode(EntitlementVerificationMode.INFORMATIONAL)
.build()

assertThat(config1).isNotEqualTo(config2)
}

@Test
fun `configurations with different dangerousSettings are not equal`() {
val config1 = PurchasesConfiguration.Builder(mockContext, "api_key")
.dangerousSettings(DangerousSettings(autoSyncPurchases = true))
.build()
val config2 = PurchasesConfiguration.Builder(mockContext, "api_key")
.dangerousSettings(DangerousSettings(autoSyncPurchases = false))
.build()

assertThat(config1).isNotEqualTo(config2)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import com.revenuecat.purchases.PostTransactionWithProductDetailsHelper
import com.revenuecat.purchases.Purchases
import com.revenuecat.purchases.PurchasesAreCompletedBy
import com.revenuecat.purchases.PurchasesAreCompletedBy.REVENUECAT
import com.revenuecat.purchases.PurchasesConfiguration
import com.revenuecat.purchases.PurchasesOrchestrator
import com.revenuecat.purchases.PurchasesState
import com.revenuecat.purchases.PurchasesStateCache
Expand Down Expand Up @@ -84,8 +85,10 @@ class SubscriberAttributesPurchasesTests {
postTransactionHelper,
)

val context = mockk<Application>(relaxed = true).also { applicationMock = it }

val purchasesOrchestrator = PurchasesOrchestrator(
application = mockk<Application>(relaxed = true).also { applicationMock = it },
application = context,
backingFieldAppUserID = appUserId,
backend = backendMock,
billing = billingWrapperMock,
Expand All @@ -106,6 +109,7 @@ class SubscriberAttributesPurchasesTests {
paywallPresentedCache = PaywallPresentedCache(),
purchasesStateCache = PurchasesStateCache(PurchasesState()),
dispatcher = SyncDispatcher(),
configuration = PurchasesConfiguration.Builder(context, "mock-api-key").build(),
)

underTest = Purchases(purchasesOrchestrator)
Expand Down