Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[in_app_purchase] migrate playing billing library to v3 #3636

Merged
merged 12 commits into from
Mar 1, 2021

Conversation

cyanglaz
Copy link
Contributor

@cyanglaz cyanglaz commented Feb 26, 2021

This PR migrates the play billing library to V3. Several breaking changes are introduced due to the breaking changes in PBL v3.

Fixes flutter/flutter#75204

The release note can be found here, which can be used as a reference for updated APIs in this PR. https://developer.android.com/google/play/billing/release-notes#3-0

Pre-launch Checklist

  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@cyanglaz cyanglaz requested review from stuartmorgan-g and mklim and removed request for LHLL February 26, 2021 03:36
testImplementation 'junit:junit:4.12'
testImplementation 'org.mockito:mockito-core:2.17.0'
testImplementation 'org.mockito:mockito-core:3.6.0'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is required to mock the objects in the new PBL.

@@ -286,7 +289,6 @@ public void launchBillingFlow_ok_null_AccountId() {
verify(mockBillingClient).launchBillingFlow(any(), billingFlowParamsCaptor.capture());
BillingFlowParams params = billingFlowParamsCaptor.getValue();
assertEquals(params.getSku(), skuId);
assertNull(params.getAccountId());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

getAccountId is not available anymore so we can't really test this value, however, we can still test if the call crashes, so ill leave the test here

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you rename the test then, to better explain what it's doing now? (Probably needs a comment in addition to a new name.)

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

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

We don't need any new tests to cover new functionality related to the added arguments?

@@ -155,8 +155,13 @@ class BillingClient {
/// The [skuDetails] needs to have already been fetched in a [querySkuDetails]
/// call. The [accountId] is an optional hashed string associated with the user
/// that's unique to your app. It's used by Google to detect unusual behavior.
/// Do not pass in a cleartext [accountId], use your developer ID, or use the
/// user's Google ID for this field.
/// Do not pass in a cleartext [accountId], Do not use this field to store any Personally Identifiable Information (PII)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: s/Do not use/and do not use/

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

/// such as emails in cleartext. Attempting to store PII in this field will result in purchases being blocked.
/// Google Play recommends that you use either encryption or a one-way hash to generate an obfuscated identifier to send to Google Play.
///
/// Specifies an optional [obfuscatedProfileId] that is uniquely associated with the user's profile in your app.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Fix double space after ]

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


* Migrate to Google Billing Library 3.0
* Add `obfuscatedProfileId`, `purchaseToken` in [BillingClientWrapper.launchBillingFlow].
* Removed `developerPayload` in [BillingClientWrapper.acknowledgePurchase], [BillingClientWrapper.consumeAsync], [InAppPurchaseConnection.completePurchase], [InAppPurchaseConnection.consumePurchase].
Copy link
Contributor

Choose a reason for hiding this comment

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

These aren't breaking changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh they are. Fixed

* Add `obfuscatedProfileId`, `purchaseToken` in [BillingClientWrapper.launchBillingFlow].
* Removed `developerPayload` in [BillingClientWrapper.acknowledgePurchase], [BillingClientWrapper.consumeAsync], [InAppPurchaseConnection.completePurchase], [InAppPurchaseConnection.consumePurchase].
* **Breaking Change**
* Removed `isRewarded` from [SkuDetailsWrapper].
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any useful context to provide about why they are removed? E.g., link to discussion of removal in the SDK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@@ -286,7 +289,6 @@ public void launchBillingFlow_ok_null_AccountId() {
verify(mockBillingClient).launchBillingFlow(any(), billingFlowParamsCaptor.capture());
BillingFlowParams params = billingFlowParamsCaptor.getValue();
assertEquals(params.getSku(), skuId);
assertNull(params.getAccountId());
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you rename the test then, to better explain what it's doing now? (Probably needs a comment in addition to a new name.)

@@ -171,25 +176,31 @@ class BillingClient {
/// [`BillingFlowParams`](https://developer.android.com/reference/com/android/billingclient/api/BillingFlowParams)
/// instance by [setting the given
/// skuDetails](https://developer.android.com/reference/com/android/billingclient/api/BillingFlowParams.Builder.html#setskudetails)
/// and [the given
/// accountId](https://developer.android.com/reference/com/android/billingclient/api/BillingFlowParams.Builder.html#setAccountId(java.lang.String)).
/// , [the given
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this is hard to read; let's put the comma on the previous line.

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

///
/// When this method is called to purchase a subscription, an optional `oldSku`
/// can be passed in. This will tell Google Play that rather than purchasing a new subscription,
/// the user needs to upgrade/downgrade the existing subscription.
/// The [oldSku](https://developer.android.com/reference/com/android/billingclient/api/BillingFlowParams.Builder#setoldsku) is the SKU id that the user is upgrading or downgrading from.
/// The [oldSku](https://developer.android.com/reference/com/android/billingclient/api/BillingFlowParams.Builder#setoldsku) and [purchaseToken] are the SKU id and purchase token that the user is upgrading or downgrading from.
/// [purchaseToken] must not be `null` if [oldSku] is not `null`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be asserted.

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

@@ -120,6 +120,8 @@ class PurchaseWrapper {
/// The payload specified by the developer when the purchase was acknowledged or consumed.
///
/// The value is `null` if it wasn't specified when the purchase was acknowledged or consumed.
/// The `developerPayload` is removed from [BillingClientWrapper.acknowledgePurchase], [BillingClientWrapper.consumeAsync], [InAppPurchaseConnection.completePurchase], [InAppPurchaseConnection.consumePurchase]
/// after plugin version `0.5.0`. As a result, this will be `null` for new purchases happened after `0.5.0`.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/happened after/that happen after updating to/

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

final String introductoryPriceCycles;
/// The number of subscription billing periods for which the user will be given the introductory price, such as 3.
/// Returns 0 if the SKU is not a subscription or doesn't have an introductory period.
/// Returns -1 if the value is not available.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "not available" mean as distinct from "doesn't have an introductory period"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be the same. Fixed.

Copy link
Contributor Author

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

@stuartmorgan Updated per your comments PTAL


* Migrate to Google Billing Library 3.0
* Add `obfuscatedProfileId`, `purchaseToken` in [BillingClientWrapper.launchBillingFlow].
* Removed `developerPayload` in [BillingClientWrapper.acknowledgePurchase], [BillingClientWrapper.consumeAsync], [InAppPurchaseConnection.completePurchase], [InAppPurchaseConnection.consumePurchase].
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh they are. Fixed

* Add `obfuscatedProfileId`, `purchaseToken` in [BillingClientWrapper.launchBillingFlow].
* Removed `developerPayload` in [BillingClientWrapper.acknowledgePurchase], [BillingClientWrapper.consumeAsync], [InAppPurchaseConnection.completePurchase], [InAppPurchaseConnection.consumePurchase].
* **Breaking Change**
* Removed `isRewarded` from [SkuDetailsWrapper].
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@@ -286,7 +289,6 @@ public void launchBillingFlow_ok_null_AccountId() {
verify(mockBillingClient).launchBillingFlow(any(), billingFlowParamsCaptor.capture());
BillingFlowParams params = billingFlowParamsCaptor.getValue();
assertEquals(params.getSku(), skuId);
assertNull(params.getAccountId());
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

@@ -155,8 +155,13 @@ class BillingClient {
/// The [skuDetails] needs to have already been fetched in a [querySkuDetails]
/// call. The [accountId] is an optional hashed string associated with the user
/// that's unique to your app. It's used by Google to detect unusual behavior.
/// Do not pass in a cleartext [accountId], use your developer ID, or use the
/// user's Google ID for this field.
/// Do not pass in a cleartext [accountId], Do not use this field to store any Personally Identifiable Information (PII)
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

/// such as emails in cleartext. Attempting to store PII in this field will result in purchases being blocked.
/// Google Play recommends that you use either encryption or a one-way hash to generate an obfuscated identifier to send to Google Play.
///
/// Specifies an optional [obfuscatedProfileId] that is uniquely associated with the user's profile in your app.
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

@@ -171,25 +176,31 @@ class BillingClient {
/// [`BillingFlowParams`](https://developer.android.com/reference/com/android/billingclient/api/BillingFlowParams)
/// instance by [setting the given
/// skuDetails](https://developer.android.com/reference/com/android/billingclient/api/BillingFlowParams.Builder.html#setskudetails)
/// and [the given
/// accountId](https://developer.android.com/reference/com/android/billingclient/api/BillingFlowParams.Builder.html#setAccountId(java.lang.String)).
/// , [the given
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

///
/// When this method is called to purchase a subscription, an optional `oldSku`
/// can be passed in. This will tell Google Play that rather than purchasing a new subscription,
/// the user needs to upgrade/downgrade the existing subscription.
/// The [oldSku](https://developer.android.com/reference/com/android/billingclient/api/BillingFlowParams.Builder#setoldsku) is the SKU id that the user is upgrading or downgrading from.
/// The [oldSku](https://developer.android.com/reference/com/android/billingclient/api/BillingFlowParams.Builder#setoldsku) and [purchaseToken] are the SKU id and purchase token that the user is upgrading or downgrading from.
/// [purchaseToken] must not be `null` if [oldSku] is not `null`.
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

@@ -120,6 +120,8 @@ class PurchaseWrapper {
/// The payload specified by the developer when the purchase was acknowledged or consumed.
///
/// The value is `null` if it wasn't specified when the purchase was acknowledged or consumed.
/// The `developerPayload` is removed from [BillingClientWrapper.acknowledgePurchase], [BillingClientWrapper.consumeAsync], [InAppPurchaseConnection.completePurchase], [InAppPurchaseConnection.consumePurchase]
/// after plugin version `0.5.0`. As a result, this will be `null` for new purchases happened after `0.5.0`.
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

final String introductoryPriceCycles;
/// The number of subscription billing periods for which the user will be given the introductory price, such as 3.
/// Returns 0 if the SKU is not a subscription or doesn't have an introductory period.
/// Returns -1 if the value is not available.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be the same. Fixed.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM with final nit.

@cyanglaz cyanglaz added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Mar 1, 2021
@fluttergithubbot fluttergithubbot merged commit 088bdee into flutter:master Mar 1, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 1, 2021
@cyanglaz cyanglaz deleted the iap_pbl_v3 branch March 2, 2021 01:20
NickalasB added a commit to NickalasB/plugins that referenced this pull request Mar 3, 2021
* master:
  Adopt Xcode 12 for podspec lints (flutter#3653)
  Run static analyzer during xctest (flutter#3667)
  [google_maps_flutter_web] update min flutter sdk version to 1.20.0 (flutter#3662)
  [image_picker] Run CocoaPods iOS tests in RunnerUITests target (flutter#3663)
  [webview_flutter] Run CocoaPods iOS tests in RunnerUITests target (flutter#3664)
  [device_info] Enable NNBD for unit test (flutter#3658)
  remove unused plugin (flutter#3661)
  [android_intent] move unit test to nullsafety (flutter#3659)
  [url_launcher] Migrate unit tests to NNBD (flutter#3657)
  [share] Migrate unit tests to null-safety. (flutter#3660)
  [connectivity_for_web] Migration to null-safety. (flutter#3652)
  [camera] Stable release for null safety. (flutter#3641)
  [in_app_purchase] fix plugin version (flutter#3654)
  Move plugin tool tests over (flutter#3606)
  [in_app_purchase] migrate playing billing library to v3 (flutter#3636)
  Update plugin_platform_interface min version (flutter#3650)

# Conflicts:
#	packages/webview_flutter/CHANGELOG.md
#	packages/webview_flutter/example/ios/Runner.xcodeproj/project.pbxproj
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes p: in_app_purchase platform-android waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[in_app_purchase] Migrate Android Billing Client to 3.0.0
3 participants