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

[In_App_Purchase] Improve testability #2016

Merged
mklim merged 14 commits into from
Sep 3, 2019
Merged

[In_App_Purchase] Improve testability #2016

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
52cd0a0
Do not register plugin if Activity is null.
juliocbcotta Aug 26, 2019
da055d5
Revert unrelated changes.
juliocbcotta Aug 26, 2019
bcf733a
Adds UnitTest for registration.
juliocbcotta Aug 26, 2019
ed97762
Improved testability.
juliocbcotta Aug 26, 2019
469a7ac
Updated CHANGELOG.md and pubspec.yaml
juliocbcotta Aug 26, 2019
cdaea15
Allow background registration and adds check for Activity nullability…
juliocbcotta Aug 27, 2019
65a6a1a
update tests and changelog.
juliocbcotta Aug 27, 2019
c673a21
Merge branch 'plugin/in_app_purchase_null_fix' into plugin/in_app_pur…
juliocbcotta Aug 27, 2019
ab48f81
Merge branch 'master' into plugin/in_app_purchase_improve_testability
juliocbcotta Aug 29, 2019
daa9a07
Merge branch 'master' into plugin/in_app_purchase_improve_testability
juliocbcotta Aug 31, 2019
e026980
removed empty line from CHANGELOG.md
juliocbcotta Aug 31, 2019
739e199
Extract BillingClientFactory to an interface to make testing simpler.
juliocbcotta Aug 31, 2019
a07f6e2
fix file formatting
juliocbcotta Aug 31, 2019
6eabb6c
formatted file using clang-format-7
juliocbcotta Sep 3, 2019
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
5 changes: 5 additions & 0 deletions packages/in_app_purchase/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.2.1+3

* Android : Improved testability.

Comment thread
mklim marked this conversation as resolved.

## 0.2.1+2

* Android: Require a non-null Activity to use the `launchBillingFlow` method.
Expand Down
14 changes: 14 additions & 0 deletions ...purchase/android/src/main/java/io/flutter/plugins/inapppurchase/BillingClientFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package io.flutter.plugins.inapppurchase;

import android.content.Context;
import com.android.billingclient.api.BillingClient;
import io.flutter.plugin.common.MethodChannel;

public class BillingClientFactory {

@mklim mklim Aug 30, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So this works and it's definitely an improvement over the current code. But there's some subjective style nits I have with this approach.

  • It's not final, so it could be subclassed in unexpected ways in the future. https://stackoverflow.com/a/218761. I think we need this to be non-final for it to be mockable, which leads to...
  • When this is used in the test, it's mocked and then the mock is returning the other mock BillingClient. That totally works but it's usually better to use real objects whenever possible.

It adds a little extra overhead, but I'd prefer that this were an interface that the test had it's own implementation instead.

interface BillingClientFactory {
  BillingClient createBillingClient(Context context, MethodChannel channel);
}

// Instantiated in InAppPurchasePlugin#registerWith
class BillingClientFactoryImpl implements BillingClientFactory {
  public BillingClient createBillingClient(Context context, MethodChannel channel) {
    return BillingClient.newBuilder(context)
          .setListener(new PluginPurchaseListener(channel))
          .build();
  }
}

And then the tests could have a real implementation of this that returned the mock:

  @Before
  public void setUp() {
    MockitoAnnotations.initMocks(this);

    BillingClientFactory factory = (Context context, MethodChannel channel) -> mockBillingClient;
    plugin = new InAppPurchasePlugin(factory, registrar, mockMethodChannel);
  }

What do you think?

@juliocbcotta juliocbcotta Aug 31, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think all your suggestions are fair.
Mockito v2 can mock final classes! \o/
https://stackoverflow.com/a/40018295/2742962
but it comes with some overhead /o\
As we control the abstraction, let's go with the interface. :-D


public BillingClient createBillingClient(Context context, MethodChannel channel) {
return BillingClient.newBuilder(context)
.setListener(new PluginPurchaseListener(channel))
.build();
}
}
45 changes: 9 additions & 36 deletions ..._purchase/android/src/main/java/io/flutter/plugins/inapppurchase/InAppPurchasePlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.android.billingclient.api.ConsumeResponseListener;
import com.android.billingclient.api.Purchase;
import com.android.billingclient.api.PurchaseHistoryResponseListener;
import com.android.billingclient.api.PurchasesUpdatedListener;
import com.android.billingclient.api.SkuDetails;
import com.android.billingclient.api.SkuDetailsParams;
import com.android.billingclient.api.SkuDetailsResponseListener;
Expand All @@ -37,6 +36,7 @@
public class InAppPurchasePlugin implements MethodCallHandler {
private static final String TAG = "InAppPurchasePlugin";
private @Nullable BillingClient billingClient;
private final BillingClientFactory factory;
private final Registrar registrar;
private final Context applicationContext;
private final MethodChannel channel;
Expand Down Expand Up @@ -69,12 +69,17 @@ static final class MethodNames {
public static void registerWith(Registrar registrar) {
final MethodChannel channel =
new MethodChannel(registrar.messenger(), "plugins.flutter.io/in_app_purchase");
channel.setMethodCallHandler(new InAppPurchasePlugin(registrar, channel));

final BillingClientFactory factory = new BillingClientFactory();
final InAppPurchasePlugin plugin = new InAppPurchasePlugin(factory, registrar, channel);
channel.setMethodCallHandler(plugin);
}

public InAppPurchasePlugin(Registrar registrar, MethodChannel channel) {
public InAppPurchasePlugin(
BillingClientFactory factory, Registrar registrar, MethodChannel channel) {
this.applicationContext = registrar.context();
this.registrar = registrar;
this.factory = factory;
this.channel = channel;
}

Expand Down Expand Up @@ -112,18 +117,9 @@ public void onMethodCall(MethodCall call, Result result) {
}
}

@VisibleForTesting
/*package*/ InAppPurchasePlugin(
Registrar registrar, @Nullable BillingClient billingClient, MethodChannel channel) {
this.billingClient = billingClient;
this.channel = channel;
this.applicationContext = registrar.context();
this.registrar = registrar;
}

private void startConnection(final int handle, final Result result) {
if (billingClient == null) {
billingClient = buildBillingClient(applicationContext, channel);
billingClient = factory.createBillingClient(applicationContext, channel);
}

billingClient.startConnection(
Expand Down Expand Up @@ -282,27 +278,4 @@ private boolean billingClientError(Result result) {
result.error("UNAVAILABLE", "BillingClient is unset. Try reconnecting.", null);
return true;
}

private static BillingClient buildBillingClient(Context context, MethodChannel channel) {
return BillingClient.newBuilder(context)
.setListener(new PluginPurchaseListener(channel))
.build();
}

@VisibleForTesting
/*package*/ static class PluginPurchaseListener implements PurchasesUpdatedListener {
private final MethodChannel channel;

PluginPurchaseListener(MethodChannel channel) {
this.channel = channel;
}

@Override
public void onPurchasesUpdated(int responseCode, @Nullable List<Purchase> purchases) {
final Map<String, Object> callbackArgs = new HashMap<>();
callbackArgs.put("responseCode", responseCode);
callbackArgs.put("purchasesList", fromPurchasesList(purchases));
channel.invokeMethod(MethodNames.ON_PURCHASES_UPDATED, callbackArgs);
}
}
}
27 changes: 27 additions & 0 deletions ...rchase/android/src/main/java/io/flutter/plugins/inapppurchase/PluginPurchaseListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package io.flutter.plugins.inapppurchase;

import static io.flutter.plugins.inapppurchase.Translator.fromPurchasesList;

import androidx.annotation.Nullable;
import com.android.billingclient.api.Purchase;
import com.android.billingclient.api.PurchasesUpdatedListener;
import io.flutter.plugin.common.MethodChannel;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

class PluginPurchaseListener implements PurchasesUpdatedListener {
private final MethodChannel channel;

PluginPurchaseListener(MethodChannel channel) {
this.channel = channel;
}

@Override
public void onPurchasesUpdated(int responseCode, @Nullable List<Purchase> purchases) {
final Map<String, Object> callbackArgs = new HashMap<>();
callbackArgs.put("responseCode", responseCode);
callbackArgs.put("purchasesList", fromPurchasesList(purchases));
channel.invokeMethod(InAppPurchasePlugin.MethodNames.ON_PURCHASES_UPDATED, callbackArgs);
}
}
36 changes: 20 additions & 16 deletions ...e/android/app/src/test/java/io/flutter/plugins/inapppurchase/InAppPurchasePluginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import static org.mockito.Mockito.when;

import android.app.Activity;
import android.content.Context;
import androidx.annotation.Nullable;
import com.android.billingclient.api.BillingClient;
import com.android.billingclient.api.BillingClient.BillingResponse;
Expand All @@ -43,12 +42,10 @@
import com.android.billingclient.api.SkuDetails;
import com.android.billingclient.api.SkuDetailsParams;
import com.android.billingclient.api.SkuDetailsResponseListener;
import io.flutter.plugin.common.BinaryMessenger;
import io.flutter.plugin.common.MethodCall;
import io.flutter.plugin.common.MethodChannel;
import io.flutter.plugin.common.MethodChannel.Result;
import io.flutter.plugin.common.PluginRegistry;
import io.flutter.plugins.inapppurchase.InAppPurchasePlugin.PluginPurchaseListener;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -60,20 +57,20 @@
import org.mockito.Spy;

public class InAppPurchasePluginTest {
InAppPurchasePlugin plugin;
private InAppPurchasePlugin plugin;
@Mock BillingClient mockBillingClient;
@Mock MethodChannel mockMethodChannel;
@Spy Result result;
@Mock PluginRegistry.Registrar registrar;
@Mock Activity activity;
@Mock BinaryMessenger messenger;
@Mock Context context;
@Mock BillingClientFactory factory;

@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
when(registrar.context()).thenReturn(context);
plugin = new InAppPurchasePlugin(registrar, mockBillingClient, mockMethodChannel);

when(factory.createBillingClient(any(), any())).thenReturn(mockBillingClient);
plugin = new InAppPurchasePlugin(factory, registrar, mockMethodChannel);
}

@Test
Expand All @@ -85,6 +82,7 @@ public void invalidMethod() {

@Test
public void isReady_true() {
mockStartConnection();
MethodCall call = new MethodCall(IS_READY, null);
when(mockBillingClient.isReady()).thenReturn(true);
plugin.onMethodCall(call, result);
Expand All @@ -93,6 +91,7 @@ public void isReady_true() {

@Test
public void isReady_false() {
mockStartConnection();
MethodCall call = new MethodCall(IS_READY, null);
when(mockBillingClient.isReady()).thenReturn(false);
plugin.onMethodCall(call, result);
Expand All @@ -113,14 +112,7 @@ public void isReady_clientDisconnected() {

@Test
public void startConnection() {
Map<String, Integer> arguments = new HashMap<>();
arguments.put("handle", 1);
MethodCall call = new MethodCall(START_CONNECTION, arguments);
ArgumentCaptor<BillingClientStateListener> captor =
ArgumentCaptor.forClass(BillingClientStateListener.class);
doNothing().when(mockBillingClient).startConnection(captor.capture());

plugin.onMethodCall(call, result);
ArgumentCaptor<BillingClientStateListener> captor = mockStartConnection();
verify(result, never()).success(any());
captor.getValue().onBillingSetupFinished(100);

Expand Down Expand Up @@ -452,6 +444,18 @@ public void consumeAsync() {
verify(result, times(1)).success(responseCode);
}

private ArgumentCaptor<BillingClientStateListener> mockStartConnection() {
Map<String, Integer> arguments = new HashMap<>();
arguments.put("handle", 1);
MethodCall call = new MethodCall(START_CONNECTION, arguments);
ArgumentCaptor<BillingClientStateListener> captor =
ArgumentCaptor.forClass(BillingClientStateListener.class);
doNothing().when(mockBillingClient).startConnection(captor.capture());

plugin.onMethodCall(call, result);
return captor;
}

private void establishConnectedBillingClient(
@Nullable Map<String, Integer> arguments, @Nullable Result result) {
if (arguments == null) {
Expand Down
2 changes: 1 addition & 1 deletion packages/in_app_purchase/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: in_app_purchase
description: A Flutter plugin for in-app purchases. Exposes APIs for making in-app purchases through the App Store and Google Play.
author: Flutter Team <flutter-dev@googlegroups.com>
homepage: https://github.com/flutter/plugins/tree/master/packages/in_app_purchase
version: 0.2.1+2
version: 0.2.1+3


dependencies:
Expand Down