[in_app_purchase_android] Implement `BillingClient` connection management and introduce `BillingClientManager` #6309

SynSzakala · 2022-08-24T16:05:11Z

This PR adds management of BillingClient connection to Android implementation of InAppPurchase. This should fix issues caused by lost connections.

New and previously existing connection management is extracted to a new component, BillingClientManager.
This component is exported in billing_client_wrappers.dart, so that it can also be used when interacting with BillingClient directly.

Fixes flutter/flutter#110663

Pre-launch Checklist

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

…econnect

dgjamaster · 2022-09-07T00:11:38Z

I look forward to this PR being merged.
Thanks!!

stuartmorgan-g · 2022-09-20T15:17:53Z

@GaryQian Ping on this review.

GaryQian · 2022-09-28T21:34:47Z

...purchase/in_app_purchase_android/lib/src/billing_client_wrappers/billing_client_manager.dart

@@ -0,0 +1,147 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.


You should document in BillingClient the drawbacks of using the BillingClient class directly and point them to the newly recommended way of handling operations using this new BillingClientManager class.

GaryQian · 2022-09-28T21:39:43Z

...purchase/in_app_purchase_android/lib/src/billing_client_wrappers/billing_client_manager.dart

+    return await block(client);
+  }
+
+  /// Ends connection to the [BillingClient].


Document when/under what conditions dispose should be called.

GaryQian · 2022-09-28T21:45:55Z

...pp_purchase/in_app_purchase_android/lib/src/billing_client_wrappers/sku_details_wrapper.dart

 const String kInvalidBillingResultErrorMessage =
    'Invalid billing result map from method channel.';

+/// Abstraction of result of [BillingClient] operation that includes


Is this class in the correct file? I expected this to live in the billing_client_manager.dart file.

packages/in_app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform.dart

GaryQian · 2022-09-29T00:28:47Z

packages/in_app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform.dart

      return purchaseDetails;
    }

+    final InAppPurchaseAndroidPlatformAddition addition =


Avoid unnecessary refactors

GaryQian · 2022-09-29T04:13:37Z

...purchase/in_app_purchase_android/lib/src/billing_client_wrappers/billing_client_manager.dart

+
+  /// [BillingClient] instance managed by this [BillingClientManager].
+  ///
+  /// This field should not be accessed outside of test code.


This sentence is redundant as @VisibleForTesting will enforce preventing its usage outside of tests.

...purchase/in_app_purchase_android/lib/src/billing_client_wrappers/billing_client_manager.dart

…econnect # Conflicts: # packages/in_app_purchase/in_app_purchase_android/CHANGELOG.md # packages/in_app_purchase/in_app_purchase_android/pubspec.yaml

GaryQian · 2022-10-05T17:38:09Z

...rchase/in_app_purchase_android/test/billing_client_wrappers/billing_client_manager_test.dart

+    test('connects on initialization', () {
+      expect(stubPlatform.countPreviousCalls(startConnectionCall), equals(1));
+    });
+    test('waits for connection before executing the operations', () {


nit: newline between tests

GaryQian · 2022-10-05T17:38:14Z

...rchase/in_app_purchase_android/test/billing_client_wrappers/billing_client_manager_test.dart

+      expect(runCalled, equals(true));
+      expect(runRawCalled, equals(true));
+    });
+    test('re-connects when client sends onBillingServiceDisconnected', () {


nit: newline between tests

GaryQian · 2022-10-05T17:38:20Z

...rchase/in_app_purchase_android/test/billing_client_wrappers/billing_client_manager_test.dart

+      );
+      expect(stubPlatform.countPreviousCalls(startConnectionCall), equals(2));
+    });
+    test(


nit: newline between tests

GaryQian · 2022-10-05T17:38:27Z

...rchase/in_app_purchase_android/test/billing_client_wrappers/billing_client_manager_test.dart

+        expect(result.responseCode, equals(BillingResponse.ok));
+      },
+    );
+    test('does not re-connect when disposed', () {


nit: newline between tests

GaryQian · 2022-10-05T17:39:55Z

...ages/in_app_purchase/in_app_purchase_android/test/in_app_purchase_android_platform_test.dart

+      );
+      expect(stubPlatform.countPreviousCalls(startConnectionCall), equals(2));
+    });
+    test(


nit: newline between tests

GaryQian · 2022-10-05T17:39:59Z

...ages/in_app_purchase/in_app_purchase_android/test/in_app_purchase_android_platform_test.dart

      //await iapAndroidPlatform.isAvailable();
      expect(stubPlatform.countPreviousCalls(startConnectionCall), equals(1));
    });
+    test('re-connects when client sends onBillingServiceDisconnected', () {


nit: newline between tests

GaryQian · 2022-10-06T20:47:23Z

...pp_purchase/in_app_purchase_android/lib/src/billing_client_wrappers/sku_details_wrapper.dart

 import 'package:json_annotation/json_annotation.dart';

-import 'billing_client_wrapper.dart';
+import '../../billing_client_wrappers.dart';


Prefer directly importing billing_client_manager.dart over the export file which contains extraneous imports.

GaryQian · 2022-10-06T20:50:09Z

@SynSzakala Overall looks good, just a few small nits. Let me know when you address them!

GaryQian · 2022-10-25T17:34:42Z

@SynSzakala It looks like there are some CLA signing issues where your secondary email needs to accept the CLA, or you can amend the commit that used your alternate email address and force-push.

GaryQian

LGTM! Thanks for your contribution. Once you address the CLA issue, we can move forwards.

…llingResponse

…econnect

SynSzakala · 2022-11-02T18:39:36Z

@GaryQian Everything should be ready for merge now.

olof-dev · 2022-11-20T21:56:44Z

Thanks for this PR! The issue with lost connections leading to refunded purchases is hitting us pretty hard in production, so this is much appreciated.

…econnect

SynSzakala · 2022-11-23T11:19:13Z

@stuartmorgan @GaryQian Is there anything preventing us from merging this?

stuartmorgan-g

@SynSzakala Could you address all of the open comments from @GaryQian's review, and fix the analysis issues that are causing repo_checks to fail?

Once that's done I can approve this for landing.

stuartmorgan-g · 2022-12-07T15:53:00Z

packages/in_app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform.dart


  Future<List<PurchaseDetails>> _getPurchaseDetailsFromResult(
-      PurchasesResultWrapper resultWrapper) async {
+    PurchasesResultWrapper resultWrapper,


Why have you added commas everywhere? Changing formatting of lines that you aren't otherwise changing makes the diff (and git blame) more confusing.

GaryQian · 2023-01-05T19:43:56Z

@SynSzakala When you get a chance to fix the failing analyzer checks, we can move forwards with this. Thanks!

olof-dev · 2023-01-19T16:37:07Z

@SynSzakala It looks like the analyzer complaints are just warnings to do with 4 unneeded awaits. I was gonna send you a PR that deletes them, but I realized that there might then be CLA-type stuff to deal with. In any case, thanks again for this PR - it's substantial!

…ges.

SynSzakala · 2023-02-03T11:39:48Z

@GaryQian @stuartmorgan Cleared all comments (including unnecessary refactors), fixed analyzer issues. Looking forward to merge this!

…ndroid_billing_client_reconnect # Conflicts: # packages/in_app_purchase/in_app_purchase_android/CHANGELOG.md

GaryQian

LGTM! Thanks

…econnect

SynSzakala · 2023-02-08T10:51:08Z

@stuartmorgan @GaryQian Are there any additional steps needed to merge this? I don't have the access to add "autosubmit" label as suggested in Tree Hygiene

stuartmorgan-g · 2023-02-10T16:21:48Z

Are there any additional steps needed to merge this?

This needs a second review, per https://github.com/flutter/flutter/wiki/Tree-hygiene#getting-a-code-review

@bparrishMines Could you do the second review here, as the ecosystem-side codeowner for this plugin?

stuartmorgan-g · 2023-02-22T15:56:20Z

We've just completed the migration of the plugin code to the flutter/packages repository, as described in https://flutter.dev/go/flutter-plugins-repo-migration, and this repository is now being archived. Unfortunately that means that all in-progress PRs here must be moved to flutter/packages.

Please see our instructions for an explanation of how to move your PR, and if you have any issues moving your PR please don't hesitate to reach out in the #hackers-ecosystem channel in Discord.

Our apologies that your PR was caught in this one-time transition. We're aware that it's disruptive in the short term, and appreciate your help in getting us to a better long-term state!

SynSzakala added 2 commits August 22, 2022 13:37

[in_app_purchases_android_platform] Add BillingClient reconnecting logic

4a89c13

[in_app_purchases_android_platform] Extract BillingClientManager

c12ca18

github-actions bot added p: in_app_purchase platform-android labels Aug 24, 2022

SynSzakala added 3 commits August 31, 2022 13:49

Merge branch 'main' into fix_in_app_purchase_android_billing_client_r…

45f2e1c

…econnect

[in_app_purchases_android] Update changelog

9f921df

[in_app_purchases_android] Update pubspec.yaml

732cda1

SynSzakala changed the title ~~Draft: Fix in app purchase android billing client reconnect~~ [in_app_purchase_android] Implement BillingClient connection management and introduce BillingClientManager Aug 31, 2022

SynSzakala marked this pull request as ready for review August 31, 2022 15:17

SynSzakala requested a review from GaryQian as a code owner August 31, 2022 15:17

stuartmorgan-g self-requested a review September 1, 2022 20:05

GaryQian reviewed Sep 28, 2022

View reviewed changes

GaryQian reviewed Sep 29, 2022

View reviewed changes

packages/in_app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform.dart Outdated Show resolved Hide resolved

GaryQian reviewed Sep 29, 2022

View reviewed changes

...purchase/in_app_purchase_android/lib/src/billing_client_wrappers/billing_client_manager.dart Show resolved Hide resolved

Merge branch 'main' into fix_in_app_purchase_android_billing_client_r…

39e5ccf

…econnect # Conflicts: # packages/in_app_purchase/in_app_purchase_android/CHANGELOG.md # packages/in_app_purchase/in_app_purchase_android/pubspec.yaml

SynSzakala requested review from GaryQian and removed request for stuartmorgan-g October 4, 2022 09:34

GaryQian reviewed Oct 5, 2022

View reviewed changes

GaryQian reviewed Oct 6, 2022

View reviewed changes

SynSzakala requested a review from GaryQian October 15, 2022 15:38

SynSzakala force-pushed the fix_in_app_purchase_android_billing_client_reconnect branch from c9d6bfe to 51bb55a Compare October 15, 2022 15:43

GaryQian approved these changes Oct 25, 2022

View reviewed changes

[in_app_purchases_android_platform] Enhance documentation, move HasBi…

213ec8a

…llingResponse

SynSzakala force-pushed the fix_in_app_purchase_android_billing_client_reconnect branch from 51bb55a to 213ec8a Compare November 2, 2022 16:17

Merge branch 'main' into fix_in_app_purchase_android_billing_client_r…

1b74c6a

…econnect

stuartmorgan-g self-requested a review November 4, 2022 14:55

Merge branch 'main' into fix_in_app_purchase_android_billing_client_r…

a2f3dc4

…econnect

stuartmorgan-g reviewed Dec 7, 2022

View reviewed changes

olof-dev mentioned this pull request Jan 19, 2023

Fix in app purchase android billing client reconnect olof-dev/flutter-plugins#1

Open

[image_picker_android] Fix analyzer warnings, remove unnecessary chan…

48ddee7

…ges.

Merge remote-tracking branch 'origin/main' into fix_in_app_purchase_a…

6bc41e0

…ndroid_billing_client_reconnect # Conflicts: # packages/in_app_purchase/in_app_purchase_android/CHANGELOG.md

SynSzakala force-pushed the fix_in_app_purchase_android_billing_client_reconnect branch from aa02afc to 6bc41e0 Compare February 3, 2023 11:42

GaryQian approved these changes Feb 3, 2023

View reviewed changes

Merge branch 'main' into fix_in_app_purchase_android_billing_client_r…

876530f

…econnect

stuartmorgan-g requested a review from bparrishMines February 10, 2023 16:20

stuartmorgan-g closed this Feb 22, 2023

SynSzakala mentioned this pull request Feb 26, 2023

[in_app_purchase_android] Implement BillingClient connection management and introduce BillingClientManager flutter/packages#3303

Merged

11 tasks

		@@ -0,0 +1,147 @@
		// Copyright 2013 The Flutter Authors. All rights reserved.

[in_app_purchase_android] Implement BillingClient connection management and introduce BillingClientManager #6309

[in_app_purchase_android] Implement BillingClient connection management and introduce BillingClientManager #6309

Uh oh!

Conversation

SynSzakala commented Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

dgjamaster commented Sep 7, 2022

Uh oh!

stuartmorgan-g commented Sep 20, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GaryQian commented Oct 6, 2022

Uh oh!

GaryQian commented Oct 25, 2022

Uh oh!

GaryQian left a comment

Choose a reason for hiding this comment

Uh oh!

SynSzakala commented Nov 2, 2022

Uh oh!

olof-dev commented Nov 20, 2022

Uh oh!

SynSzakala commented Nov 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GaryQian commented Jan 5, 2023

Uh oh!

olof-dev commented Jan 19, 2023

Uh oh!

SynSzakala commented Feb 3, 2023

Uh oh!

GaryQian left a comment

Choose a reason for hiding this comment

Uh oh!

SynSzakala commented Feb 8, 2023

Uh oh!

stuartmorgan-g commented Feb 10, 2023

Uh oh!

stuartmorgan-g commented Feb 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[in_app_purchase_android] Implement `BillingClient` connection management and introduce `BillingClientManager` #6309

[in_app_purchase_android] Implement `BillingClient` connection management and introduce `BillingClientManager` #6309

SynSzakala commented Aug 24, 2022 •

edited

Loading

SynSzakala commented Nov 23, 2022 •

edited

Loading