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

CustomEntitlementComputation: Create different PurchasesConfiguration that requires an appUserId parameter #1154

Merged
merged 7 commits into from
Jul 21, 2023

Conversation

tonidero
Copy link
Contributor

Description

This PR separates PurchasesConfiguration in 2 for the defaults and customEntitlementComputation flavor. The one in customEntitlementComputation will require an app user id, avoiding initializing the sdk with anonymous users.

import com.revenuecat.purchases.PurchasesConfiguration
import com.revenuecat.purchases.Store

class AmazonConfiguration(builder: Builder) : PurchasesConfiguration(builder) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also had to create amazon classes for each flavor... Not sure if it's needed but adding it for now

Copy link
Member

Choose a reason for hiding this comment

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

I'd be more inclined to keep Amazon only in the main flavor, unless there's a technical limitation that forces us to have it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm I thought of a way to do that, but will do it in a separate PR. For now, I'm just failing when using this in customEntitlementComputation mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in #1158

* This information will be anonymized so it can't be traced back to the end-user.
* The default value is false.
*/
fun diagnosticsEnabled(diagnosticsEnabled: Boolean) = apply {
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'm wondering, should we disable diagnostics in customEntitlementComputation mode, @aboedo?

Copy link
Member

Choose a reason for hiding this comment

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

let's keep it disabled on v1 and we can take it from there

val builder = PurchasesConfiguration.Builder(mockContext, "api").store(Store.PLAY_STORE)
Purchases.configure(builder.build())
assertThat(Purchases.sharedInstance.store).isEqualTo(Store.PLAY_STORE)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to separate these tests per flavor, now that the purchases configuration is different.

every { apiKey } returns testApiKey
every { appUserID } returns "appUserID"
every { store } returns Store.PLAY_STORE
}
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 could have split these tests... But I decided to just use a mock to not have to do that.

@codecov
Copy link

codecov bot commented Jul 20, 2023

Codecov Report

Merging #1154 (10f6340) into main (6ec7485) will increase coverage by 0.29%.
The diff coverage is 67.56%.

❗ Current head 10f6340 differs from pull request most recent head 5a3529b. Consider uploading reports for the commit 5a3529b to get more accurate results

@@            Coverage Diff             @@
##             main    #1154      +/-   ##
==========================================
+ Coverage   85.40%   85.69%   +0.29%     
==========================================
  Files         177      176       -1     
  Lines        6275     6095     -180     
  Branches      921      915       -6     
==========================================
- Hits         5359     5223     -136     
+ Misses        571      531      -40     
+ Partials      345      341       -4     
Impacted Files Coverage Δ
...revenuecat/purchases/CoroutinesExtensionsCommon.kt 80.00% <ø> (ø)
.../revenuecat/purchases/ListenerConversionsCommon.kt 60.71% <60.71%> (ø)
...n/com/revenuecat/purchases/coroutinesExtensions.kt 85.71% <85.71%> (ø)
...in/com/revenuecat/purchases/listenerConversions.kt 23.07% <100.00%> (ø)

... and 4 files with indirect coverage changes

@tonidero tonidero force-pushed the toniricodiez/sdk-3229-disallow-anonymous-users branch from 36d9eab to 4e590d0 Compare July 20, 2023 09:19
@tonidero tonidero force-pushed the toniricodiez/sdk-3223-new-custom-entitlements-purchases-class branch from 33341fa to ca42170 Compare July 20, 2023 09:47
@tonidero tonidero force-pushed the toniricodiez/sdk-3229-disallow-anonymous-users branch from 4e590d0 to c0e8cee Compare July 20, 2023 10:19
import android.content.Context
import java.util.concurrent.ExecutorService

open class PurchasesConfiguration(builder: Builder) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally I wasn't planning to copy the whole PurchasesConfiguration... But I didn't know how to have the Builder have a different constructor per flavor without duplicating everything like this :(

Copy link
Member

Choose a reason for hiding this comment

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

Could it just be a public extension declared in each flavor?

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 don't think kotlin allows to declare an inner class or a constructor as an extension :(

* Only use a Dangerous Setting if suggested by RevenueCat support team.
*/
fun dangerousSettings(dangerousSettings: DangerousSettings) = apply {
this.dangerousSettings = dangerousSettings.copy(customEntitlementComputation = true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note how I'm doing this, so in case somone using this package changes dangerous settings, we still enable customEntitlementComputation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm this makes me think why do we even have the setter in DangerousSettings? If someone in the defaults flavor enables it, they won't access the same API, and if someone in the custom entitlements flavor disables it and sets it here, it will be re-enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm the customEntitlementComputation in DangerousSettings is internal, so devs won't be able to set it up themselves. So it should always be false in the defaults flavor and true in the customEntitlementComputation flavor. Not sure if I'm missing something?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh then nevermind, I had missed that point

@tonidero tonidero marked this pull request as ready for review July 20, 2023 10:41
@tonidero tonidero requested a review from a team July 20, 2023 10:41
@tonidero tonidero force-pushed the toniricodiez/sdk-3229-disallow-anonymous-users branch from 20e21e8 to 36ee010 Compare July 20, 2023 10:55
* Only use a Dangerous Setting if suggested by RevenueCat support team.
*/
fun dangerousSettings(dangerousSettings: DangerousSettings) = apply {
this.dangerousSettings = dangerousSettings.copy(customEntitlementComputation = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm this makes me think why do we even have the setter in DangerousSettings? If someone in the defaults flavor enables it, they won't access the same API, and if someone in the custom entitlements flavor disables it and sets it here, it will be re-enabled

@tonidero tonidero force-pushed the toniricodiez/sdk-3223-new-custom-entitlements-purchases-class branch from bf5776f to 85d6dc5 Compare July 20, 2023 14:39
@tonidero tonidero force-pushed the toniricodiez/sdk-3229-disallow-anonymous-users branch 2 times, most recently from 2c28c0c to 10f6340 Compare July 21, 2023 07:17
@tonidero tonidero requested review from vegaro and aboedo July 21, 2023 07:23
) : PurchasesConfiguration.Builder(context, apiKey, appUserId) {

init {
error("AmazonConfiguration is not currently supported in the custom entitlement computation package.")
Copy link
Contributor Author

@tonidero tonidero Jul 21, 2023

Choose a reason for hiding this comment

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

Will remove this class (and the customEntitlementComputation flavor) in a followup PR

this.dangerousSettings = builder.dangerousSettings
}

open class Builder(
Copy link
Contributor Author

@tonidero tonidero Jul 21, 2023

Choose a reason for hiding this comment

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

Note that I removed the observerMode setter as well, so it won't be available in the customEntitlementComputation package. CC @aboedo

Base automatically changed from toniricodiez/sdk-3223-new-custom-entitlements-purchases-class to main July 21, 2023 14:17
@tonidero tonidero force-pushed the toniricodiez/sdk-3229-disallow-anonymous-users branch from 10f6340 to 5a3529b Compare July 21, 2023 14:18
@tonidero tonidero enabled auto-merge (squash) July 21, 2023 14:18
@tonidero tonidero merged commit c661ac4 into main Jul 21, 2023
@tonidero tonidero deleted the toniricodiez/sdk-3229-disallow-anonymous-users branch July 21, 2023 14:29
Comment on lines +57 to +72
/**
* Sets the [EntitlementVerificationMode] to perform signature verification of requests to the
* RevenueCat backend.
*
* When changing from [EntitlementVerificationMode.DISABLED] to other modes, the SDK will clear the
* CustomerInfo cache.
* This means users will need to connect to the internet to get their entitlements back.
*
* The result of the verification can be obtained from [EntitlementInfos.verification] or
* [EntitlementInfo.verification].
*
* Default mode is disabled. Please see https://rev.cat/trusted-entitlements for more info.
*/
fun entitlementVerificationMode(verificationMode: EntitlementVerificationMode) = apply {
this.verificationMode = verificationMode
}
Copy link
Member

Choose a reason for hiding this comment

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

this shouldn't be added here, it's not a part of this mode

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 see you're already removing it in #1167

aboedo added a commit that referenced this pull request Jul 24, 2023
**This is an automatic release.**

### New Features

Introduced Custom Entitlements Computation mode. 

This is new library intended for apps that will do their own entitlement
computation separate from RevenueCat. It's distributed as a separate
artifact in Maven.

Apps using this mode rely on webhooks to signal their backends to
refresh entitlements with RevenueCat.

See the [demo app for an example and usage
instructions](https://github.com/RevenueCat/purchases-android/tree/main/examples/CustomEntitlementComputationSample).

* Custom entitlements: add README and other improvements (#1167) via
Andy Boedo (@aboedo)
* Update Custom Entitlements Sample app (#1166) via Andy Boedo (@aboedo)
* purchase coroutine (#1142) via Andy Boedo (@aboedo)
* Add switchUser (#1156) via Cesar de la Vega (@vegaro)
* CustomEntitlementsComputation: disable first listener callback when
set (#1152) via Andy Boedo (@aboedo)
* CustomEntitlementsComputation: Prevent posting subscriber attributes
in post receipt (#1151) via Andy Boedo (@aboedo)
* Fix `customEntitlementComputation` library deployment (#1169) via Toni
Rico (@tonidero)
* CustomEntitlementComputation: Configure method for
customEntitlementComputation mode (#1168) via Toni Rico (@tonidero)
* Add publish system for customEntitlementComputation package (#1149)
via Cesar de la Vega (@vegaro)
* Use purchase coroutine in CustomEntitlementComputationSample (#1162)
via Cesar de la Vega (@vegaro)
* Adds CustomEntitlementComputationSample (#1160) via Cesar de la Vega
(@vegaro)
* Fix tests in customEntitlementComputation after merges (#1161) via
Toni Rico (@tonidero)
* CustomEntitlementComputation: Remove custom entitlement computation
flavor for amazon module (#1158) via Toni Rico (@tonidero)
* CustomEntitlementComputation: Generate dokka docs only for defaults
flavor (#1159) via Toni Rico (@tonidero)
* CustomEntitlementComputation: Create different PurchasesConfiguration
that requires an appUserId parameter (#1154) via Toni Rico (@tonidero)
* CustomEntitlementComputation: New Purchases class (#1153) via Toni
Rico (@tonidero)
* CustomEntitlementComputation: Disable automatic cache refresh (#1157)
via Toni Rico (@tonidero)
* Add `customEntitlementComputation` flavor (#1147) via Toni Rico
(@tonidero)
* Make `customEntitlementComputation` singular (#1148) via Toni Rico
(@tonidero)
* Disable offline entitlements in custom entitlements computation mode
(#1146) via Toni Rico (@tonidero)
* Remove integration test flavor (#1143) via Toni Rico (@tonidero)
* Add header to requests when in custom entitlement computation mode
(#1145) via Toni Rico (@tonidero)
* Add internal customEntitlementsComputation mode to app config (#1141)
via Toni Rico (@tonidero)

### New Coroutines
* `awaitPurchase` is available as a coroutine-friendly alternative to
`purchase()`. (#1142) via Andy Boedo (@aboedo)

### Dependency Updates
* Bump fastlane from 2.213.0 to 2.214.0 (#1140) via dependabot[bot]
(@dependabot[bot])

### Other changes
* CI: make all Codecov jobs informational (#1155) via Cesar de la Vega
(@vegaro)
* Creates PurchasesOrchestrator (#1144) via Cesar de la Vega (@vegaro)

---------

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Co-authored-by: Andy Boedo <andresboedo@gmail.com>
tonidero pushed a commit that referenced this pull request Jul 25, 2023
### New Features

Introduced Custom Entitlements Computation mode. 

This is new library intended for apps that will do their own entitlement
computation separate from RevenueCat. It's distributed as a separate
artifact in Maven.

Apps using this mode rely on webhooks to signal their backends to
refresh entitlements with RevenueCat.

See the [demo app for an example and usage
instructions](https://github.com/RevenueCat/purchases-android/tree/main/examples/CustomEntitlementComputationSample).

* Custom entitlements: add README and other improvements (#1167) via
Andy Boedo (@aboedo)
* Update Custom Entitlements Sample app (#1166) via Andy Boedo (@aboedo)
* purchase coroutine (#1142) via Andy Boedo (@aboedo)
* Add switchUser (#1156) via Cesar de la Vega (@vegaro)
* CustomEntitlementsComputation: disable first listener callback when
set (#1152) via Andy Boedo (@aboedo)
* CustomEntitlementsComputation: Prevent posting subscriber attributes
in post receipt (#1151) via Andy Boedo (@aboedo)
* Fix `customEntitlementComputation` library deployment (#1169) via Toni
Rico (@tonidero)
* CustomEntitlementComputation: Configure method for
customEntitlementComputation mode (#1168) via Toni Rico (@tonidero)
* Add publish system for customEntitlementComputation package (#1149)
via Cesar de la Vega (@vegaro)
* Use purchase coroutine in CustomEntitlementComputationSample (#1162)
via Cesar de la Vega (@vegaro)
* Adds CustomEntitlementComputationSample (#1160) via Cesar de la Vega
(@vegaro)
* Fix tests in customEntitlementComputation after merges (#1161) via
Toni Rico (@tonidero)
* CustomEntitlementComputation: Remove custom entitlement computation
flavor for amazon module (#1158) via Toni Rico (@tonidero)
* CustomEntitlementComputation: Generate dokka docs only for defaults
flavor (#1159) via Toni Rico (@tonidero)
* CustomEntitlementComputation: Create different PurchasesConfiguration
that requires an appUserId parameter (#1154) via Toni Rico (@tonidero)
* CustomEntitlementComputation: New Purchases class (#1153) via Toni
Rico (@tonidero)
* CustomEntitlementComputation: Disable automatic cache refresh (#1157)
via Toni Rico (@tonidero)
* Add `customEntitlementComputation` flavor (#1147) via Toni Rico
(@tonidero)
* Make `customEntitlementComputation` singular (#1148) via Toni Rico
(@tonidero)
* Disable offline entitlements in custom entitlements computation mode
(#1146) via Toni Rico (@tonidero)
* Remove integration test flavor (#1143) via Toni Rico (@tonidero)
* Add header to requests when in custom entitlement computation mode
(#1145) via Toni Rico (@tonidero)
* Add internal customEntitlementsComputation mode to app config (#1141)
via Toni Rico (@tonidero)

### New Coroutines
* `awaitPurchase` is available as a coroutine-friendly alternative to
`purchase()`. (#1142) via Andy Boedo (@aboedo)

### Dependency Updates
* Bump fastlane from 2.213.0 to 2.214.0 (#1140) via dependabot[bot]
(@dependabot[bot])

### Other changes
* CI: make all Codecov jobs informational (#1155) via Cesar de la Vega
(@vegaro)
* Creates PurchasesOrchestrator (#1144) via Cesar de la Vega (@vegaro)

---------

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Co-authored-by: Andy Boedo <andresboedo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants