From 670d1ef4ab9ef32901ae19b7805c61bff0c426cf Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Fri, 6 Dec 2013 11:54:26 -0600 Subject: [PATCH] Update billing * Add unit tests * Dont start service if user already donated - #53 --- astrid/build.gradle | 6 +- .../astrid/billing/BillingConstants.java | 1 + .../astrid/billing/BillingService.java | 19 ++- .../astrid/billing/PurchaseObserver.java | 56 +++------ .../astrid/billing/ResponseHandler.java | 12 +- .../tasks/billing/PurchaseHandler.java} | 44 +++---- .../src/test/java/org/tasks/SampleTest.java | 12 -- .../test/java/org/tasks/TestUtilities.java | 16 +++ .../tasks/billing/PurchaseHandlerTest.java | 117 ++++++++++++++++++ 9 files changed, 187 insertions(+), 96 deletions(-) rename astrid/src/main/java/{com/todoroo/astrid/billing/AstridPurchaseObserver.java => org/tasks/billing/PurchaseHandler.java} (60%) delete mode 100644 astrid/src/test/java/org/tasks/SampleTest.java create mode 100644 astrid/src/test/java/org/tasks/TestUtilities.java create mode 100644 astrid/src/test/java/org/tasks/billing/PurchaseHandlerTest.java diff --git a/astrid/build.gradle b/astrid/build.gradle index 8a6d540d5..7555ab71e 100644 --- a/astrid/build.gradle +++ b/astrid/build.gradle @@ -95,9 +95,11 @@ dependencies { compile group: 'com.google.oauth-client', name: 'google-oauth-client', version: '1.6.0-beta', transitive: false compile group: 'com.google.oauth-client', name: 'google-oauth-client-extensions', version: '1.6.0-beta', transitive: false - testCompile 'junit:junit:4.10' - testCompile 'org.robolectric:robolectric:2.3-SNAPSHOT' + testCompile group: 'junit', name: 'junit', version: '4.10' + testCompile group: 'org.robolectric', name: 'robolectric', version: '2.3-SNAPSHOT' + testCompile group: 'org.mockito', name: 'mockito-all', version: '1.9.0' // hack to get android studio to import libraries instrumentTestCompile group: 'junit', name: 'junit', version: '4.10', transitive: false instrumentTestCompile group: 'org.robolectric', name: 'robolectric', version: '2.3-SNAPSHOT', transitive: false + instrumentTestCompile group: 'org.mockito', name: 'mockito-all', version: '1.9.0', transitive: false } diff --git a/astrid/src/main/java/com/todoroo/astrid/billing/BillingConstants.java b/astrid/src/main/java/com/todoroo/astrid/billing/BillingConstants.java index caead1ddc..d34515db6 100644 --- a/astrid/src/main/java/com/todoroo/astrid/billing/BillingConstants.java +++ b/astrid/src/main/java/com/todoroo/astrid/billing/BillingConstants.java @@ -42,6 +42,7 @@ public class BillingConstants { // These are the types supported in the IAB v2 public static final String ITEM_TYPE_INAPP = "inapp"; + public static final String ITEM_TYPE_SUBSCRIPTION = "subs"; public static final String TASKS_DONATION_ITEM_ID = "tasks_donation_4_6"; diff --git a/astrid/src/main/java/com/todoroo/astrid/billing/BillingService.java b/astrid/src/main/java/com/todoroo/astrid/billing/BillingService.java index 48baf7662..65c0b6aff 100644 --- a/astrid/src/main/java/com/todoroo/astrid/billing/BillingService.java +++ b/astrid/src/main/java/com/todoroo/astrid/billing/BillingService.java @@ -15,6 +15,8 @@ import android.util.Log; import com.android.vending.billing.IMarketBillingService; import com.todoroo.astrid.billing.BillingConstants.ResponseCode; +import org.tasks.billing.PurchaseHandler; + import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedList; @@ -31,10 +33,11 @@ public class BillingService extends Service implements ServiceConnection { private static HashMap sentRequests = new HashMap<>(); - private AstridPurchaseObserver purchaseObserver; + private PurchaseObserver purchaseObserver; + private PurchaseHandler purchaseHandler; public boolean showDonateOption() { - return purchaseObserver.isBillingSupported() && !purchaseObserver.userDonated(); + return purchaseHandler.isBillingSupported() && !purchaseHandler.userDonated(); } abstract class BillingRequest { @@ -167,6 +170,7 @@ public class BillingService extends Service implements ServiceConnection { ResponseCode.valueOf(responseCode)); } boolean billingSupported = (responseCode == ResponseCode.RESULT_OK.ordinal()); + Log.d(TAG, "check billing support type: " + mProductType + ", response code: " + responseCode); ResponseHandler.checkBillingSupportedResponse(billingSupported, mProductType); return BillingConstants.BILLING_RESPONSE_INVALID_REQUEST_ID; } @@ -221,7 +225,7 @@ public class BillingService extends Service implements ServiceConnection { @Override protected void responseCodeReceived(ResponseCode responseCode) { - ResponseHandler.responseCodeReceived(this, responseCode); + Log.d(TAG, "received response code " + responseCode + " for request " + this); } } @@ -291,13 +295,15 @@ public class BillingService extends Service implements ServiceConnection { @Override protected void responseCodeReceived(ResponseCode responseCode) { - ResponseHandler.responseCodeReceived(this, responseCode); + Log.d(TAG, "received response code " + responseCode + " for request " + this); + ResponseHandler.responseCodeReceived(responseCode); } } public void setActivity(Activity activity) { attachBaseContext(activity); - purchaseObserver = new AstridPurchaseObserver(activity, this); + purchaseHandler = new PurchaseHandler(this); + purchaseObserver = new PurchaseObserver(activity, purchaseHandler); } @Override @@ -366,7 +372,7 @@ public class BillingService extends Service implements ServiceConnection { } public boolean checkBillingSupported() { - return new CheckBillingSupported(BillingConstants.ITEM_TYPE_INAPP).runRequest(); + return purchaseHandler.userDonated() || new CheckBillingSupported(BillingConstants.ITEM_TYPE_INAPP).runRequest(); } /** @@ -446,6 +452,7 @@ public class BillingService extends Service implements ServiceConnection { if (vp.notificationId != null) { notifyList.add(vp.notificationId); } + Log.d(TAG, "purchase state changed productId: " + vp.productId + ", state: " + vp.purchaseState); ResponseHandler.purchaseResponse(vp.purchaseState, vp.productId); } if (!notifyList.isEmpty()) { diff --git a/astrid/src/main/java/com/todoroo/astrid/billing/PurchaseObserver.java b/astrid/src/main/java/com/todoroo/astrid/billing/PurchaseObserver.java index 8723a71ba..910a20159 100644 --- a/astrid/src/main/java/com/todoroo/astrid/billing/PurchaseObserver.java +++ b/astrid/src/main/java/com/todoroo/astrid/billing/PurchaseObserver.java @@ -11,63 +11,35 @@ import android.util.Log; import com.todoroo.astrid.billing.BillingConstants.PurchaseState; import com.todoroo.astrid.billing.BillingConstants.ResponseCode; -import com.todoroo.astrid.billing.BillingService.RequestPurchase; -import com.todoroo.astrid.billing.BillingService.RestoreTransactions; + +import org.tasks.billing.PurchaseHandler; import java.lang.reflect.Method; -public abstract class PurchaseObserver { +public class PurchaseObserver { protected static final String TAG = "purchase-observer"; //$NON-NLS-1$ protected final Activity mActivity; - private final Handler mHandler; + private PurchaseHandler purchaseHandler; + private final Handler mHandler = new Handler(); private Method mStartIntentSender; private final Object[] mStartIntentSenderArgs = new Object[5]; private static final Class[] START_INTENT_SENDER_SIG = new Class[] { IntentSender.class, Intent.class, int.class, int.class, int.class }; - public PurchaseObserver(Activity activity, Handler handler) { + public PurchaseObserver(Activity activity, PurchaseHandler purchaseHandler) { mActivity = activity; - mHandler = handler; + this.purchaseHandler = purchaseHandler; initCompatibilityLayer(); } - public abstract void onBillingSupported(boolean supported, String type); - - public abstract void onPurchaseStateChange(PurchaseState purchaseState, String itemId); - - /** - * This is called when we receive a response code from Market for a - * RequestPurchase request that we made. This is NOT used for any - * purchase state changes. All purchase state changes are received in - * onPurchaseStateChange(PurchaseState, String, int, long). - * This is used for reporting various errors, or if the user backed out - * and didn't purchase the item. The possible response codes are: - * RESULT_OK means that the order was sent successfully to the server. - * The onPurchaseStateChange() will be invoked later (with a - * purchase state of PURCHASED or CANCELED) when the order is - * charged or canceled. This response code can also happen if an - * order for a Market-managed item was already sent to the server. - * RESULT_USER_CANCELED means that the user didn't buy the item. - * RESULT_SERVICE_UNAVAILABLE means that we couldn't connect to the - * Android Market server (for example if the data connection is down). - * RESULT_BILLING_UNAVAILABLE means that in-app billing is not - * supported yet. - * RESULT_ITEM_UNAVAILABLE means that the item this app offered for - * sale does not exist (or is not published) in the server-side - * catalog. - * RESULT_ERROR is used for any other errors (such as a server error). - */ - public abstract void onRequestPurchaseResponse(RequestPurchase request, - ResponseCode responseCode); + public void onBillingSupported(boolean supported, String type) { + purchaseHandler.onBillingSupported(supported, type); + } - /** - * This is called when we receive a response code from Android Market for a - * RestoreTransactions request that we made. A response code of - * RESULT_OK means that the request was successfully sent to the server. - */ - public abstract void onRestoreTransactionsResponse(RestoreTransactions request, - ResponseCode responseCode); + public void onRestoreTransactionsResponse(ResponseCode responseCode) { + purchaseHandler.onRestoreTransactionsResponse(responseCode); + } private void initCompatibilityLayer() { try { @@ -95,7 +67,7 @@ public abstract class PurchaseObserver { mHandler.post(new Runnable() { @Override public void run() { - onPurchaseStateChange(purchaseState, itemId); + purchaseHandler.onPurchaseStateChange(purchaseState, itemId); } }); } diff --git a/astrid/src/main/java/com/todoroo/astrid/billing/ResponseHandler.java b/astrid/src/main/java/com/todoroo/astrid/billing/ResponseHandler.java index 7667956a6..92b20a4f5 100644 --- a/astrid/src/main/java/com/todoroo/astrid/billing/ResponseHandler.java +++ b/astrid/src/main/java/com/todoroo/astrid/billing/ResponseHandler.java @@ -5,8 +5,6 @@ import android.content.Intent; import com.todoroo.astrid.billing.BillingConstants.PurchaseState; import com.todoroo.astrid.billing.BillingConstants.ResponseCode; -import com.todoroo.astrid.billing.BillingService.RequestPurchase; -import com.todoroo.astrid.billing.BillingService.RestoreTransactions; public class ResponseHandler { @@ -47,15 +45,9 @@ public class ResponseHandler { }).start(); } - public static void responseCodeReceived(RequestPurchase request, ResponseCode responseCode) { + public static void responseCodeReceived(ResponseCode responseCode) { if (sPurchaseObserver != null) { - sPurchaseObserver.onRequestPurchaseResponse(request, responseCode); - } - } - - public static void responseCodeReceived(RestoreTransactions request, ResponseCode responseCode) { - if (sPurchaseObserver != null) { - sPurchaseObserver.onRestoreTransactionsResponse(request, responseCode); + sPurchaseObserver.onRestoreTransactionsResponse(responseCode); } } } diff --git a/astrid/src/main/java/com/todoroo/astrid/billing/AstridPurchaseObserver.java b/astrid/src/main/java/org/tasks/billing/PurchaseHandler.java similarity index 60% rename from astrid/src/main/java/com/todoroo/astrid/billing/AstridPurchaseObserver.java rename to astrid/src/main/java/org/tasks/billing/PurchaseHandler.java index 39d7c62d7..01f9d6602 100644 --- a/astrid/src/main/java/com/todoroo/astrid/billing/AstridPurchaseObserver.java +++ b/astrid/src/main/java/org/tasks/billing/PurchaseHandler.java @@ -1,31 +1,29 @@ -package com.todoroo.astrid.billing; - -import android.app.Activity; -import android.os.Handler; -import android.util.Log; +package org.tasks.billing; import com.todoroo.andlib.utility.Preferences; import com.todoroo.astrid.actfm.sync.ActFmPreferenceService; +import com.todoroo.astrid.billing.BillingConstants; import com.todoroo.astrid.billing.BillingConstants.PurchaseState; import com.todoroo.astrid.billing.BillingConstants.ResponseCode; -import com.todoroo.astrid.billing.BillingService.RequestPurchase; -import com.todoroo.astrid.billing.BillingService.RestoreTransactions; +import com.todoroo.astrid.billing.BillingService; import static com.todoroo.andlib.utility.Preferences.getBoolean; import static com.todoroo.andlib.utility.Preferences.getInt; import static com.todoroo.andlib.utility.Preferences.getStringValue; -public class AstridPurchaseObserver extends PurchaseObserver { +public class PurchaseHandler { + private static final String PREF_PRODUCT_ID = ActFmPreferenceService.IDENTIFIER + "_inapp_product_id"; private static final String PREF_PURCHASE_STATE = ActFmPreferenceService.IDENTIFIER + "_inapp_purchase_state"; private static final String PREF_TRANSACTIONS_INITIALIZED = "premium_transactions_initialized"; //$NON-NLS-1$ private boolean billingSupported; + private boolean userDonated; private BillingService billingService; - public AstridPurchaseObserver(Activity activity, BillingService billingService) { - super(activity, new Handler()); + public PurchaseHandler(BillingService billingService) { this.billingService = billingService; + updateDonationStatus(); } public boolean isBillingSupported() { @@ -33,40 +31,38 @@ public class AstridPurchaseObserver extends PurchaseObserver { } public boolean userDonated() { - return BillingConstants.TASKS_DONATION_ITEM_ID.equals(getStringValue(PREF_PRODUCT_ID)) && + return userDonated; + } + + private void updateDonationStatus() { + userDonated = BillingConstants.TASKS_DONATION_ITEM_ID.equals(getStringValue(PREF_PRODUCT_ID)) && getInt(PREF_PURCHASE_STATE, -1) == PurchaseState.PURCHASED.ordinal(); } - @Override public void onBillingSupported(boolean supported, String type) { - Log.d(TAG, "onBillingSupported(" + supported + ", " + type + ")"); if (BillingConstants.ITEM_TYPE_INAPP.equals(type)) { billingSupported = supported; - if (supported && !getBoolean(PREF_TRANSACTIONS_INITIALIZED, false)) { + if (supported && !restoredTransactions()) { billingService.restoreTransactions(); } } } - @Override public void onPurchaseStateChange(PurchaseState purchaseState, final String itemId) { - Log.d(TAG, "onPurchaseStateChange(" + purchaseState + ", " + itemId + ")"); if (BillingConstants.TASKS_DONATION_ITEM_ID.equals(itemId)) { Preferences.setString(PREF_PRODUCT_ID, itemId); Preferences.setInt(PREF_PURCHASE_STATE, purchaseState.ordinal()); + updateDonationStatus(); } } - @Override - public void onRequestPurchaseResponse(RequestPurchase request, ResponseCode responseCode) { - Log.d(TAG, "onRequestPurchaseResponse(" + request + ", " + responseCode + ")"); - } - - @Override - public void onRestoreTransactionsResponse(RestoreTransactions request, ResponseCode responseCode) { - Log.d(TAG, "onRestoreTransactionsResponse(" + request + ", " + responseCode + ")"); + public void onRestoreTransactionsResponse(ResponseCode responseCode) { if (responseCode == ResponseCode.RESULT_OK) { Preferences.setBoolean(PREF_TRANSACTIONS_INITIALIZED, true); } } + + boolean restoredTransactions() { + return getBoolean(PREF_TRANSACTIONS_INITIALIZED, false); + } } diff --git a/astrid/src/test/java/org/tasks/SampleTest.java b/astrid/src/test/java/org/tasks/SampleTest.java deleted file mode 100644 index 47ee07ce9..000000000 --- a/astrid/src/test/java/org/tasks/SampleTest.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.tasks; - -import org.junit.Test; -import org.junit.runner.RunWith; -import org.robolectric.RobolectricTestRunner; - -@RunWith(RobolectricTestRunner.class) -public class SampleTest { - @Test - public void SomeTest() { - } -} diff --git a/astrid/src/test/java/org/tasks/TestUtilities.java b/astrid/src/test/java/org/tasks/TestUtilities.java new file mode 100644 index 000000000..73895432d --- /dev/null +++ b/astrid/src/test/java/org/tasks/TestUtilities.java @@ -0,0 +1,16 @@ +package org.tasks; + +import com.todoroo.astrid.utility.AstridPreferences; + +import static com.todoroo.andlib.utility.Preferences.getPrefs; +import static org.robolectric.Robolectric.getShadowApplication; + +public class TestUtilities { + public static void resetPreferences() { + getPrefs(getShadowApplication().getApplicationContext()) + .edit() + .clear() + .commit(); + AstridPreferences.setPreferenceDefaults(); + } +} diff --git a/astrid/src/test/java/org/tasks/billing/PurchaseHandlerTest.java b/astrid/src/test/java/org/tasks/billing/PurchaseHandlerTest.java new file mode 100644 index 000000000..22e42468e --- /dev/null +++ b/astrid/src/test/java/org/tasks/billing/PurchaseHandlerTest.java @@ -0,0 +1,117 @@ +package org.tasks.billing; + +import com.todoroo.astrid.billing.BillingConstants; +import com.todoroo.astrid.billing.BillingService; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.tasks.TestUtilities.resetPreferences; + +@RunWith(RobolectricTestRunner.class) +public class PurchaseHandlerTest { + + BillingService billingService; + PurchaseHandler purchaseHandler; + + @Before + public void before() { + resetPreferences(); + billingService = mock(BillingService.class); + purchaseHandler = new PurchaseHandler(billingService); + } + + @After + public void after() { + verifyNoMoreInteractions(billingService); + } + + @Test + public void userHasNotDonatedByDefault() { + assertFalse(purchaseHandler.userDonated()); + } + + @Test + public void billingNotSupportedByDefault() { + assertFalse(purchaseHandler.isBillingSupported()); + } + + @Test + public void haveNotRestoredTransactionsByDefault() { + assertFalse(purchaseHandler.restoredTransactions()); + } + + @Test + public void restoreTransactions() { + purchaseHandler.onBillingSupported(true, BillingConstants.ITEM_TYPE_INAPP); + + verify(billingService).restoreTransactions(); + } + + @Test + public void dontRestoreWhenBillingNotSupported() { + purchaseHandler.onBillingSupported(false, BillingConstants.ITEM_TYPE_INAPP); + } + + @Test + public void dontRestoreWhenAlreadyRestored() { + purchaseHandler.onRestoreTransactionsResponse(BillingConstants.ResponseCode.RESULT_OK); + purchaseHandler.onBillingSupported(true, BillingConstants.ITEM_TYPE_INAPP); + } + + @Test + public void ignoreSubscriptions() { + purchaseHandler.onBillingSupported(true, BillingConstants.ITEM_TYPE_SUBSCRIPTION); + } + + @Test + public void userDonated() { + purchaseHandler.onPurchaseStateChange(BillingConstants.PurchaseState.PURCHASED, BillingConstants.TASKS_DONATION_ITEM_ID); + + assertTrue(purchaseHandler.userDonated()); + } + + @Test + public void ignoreFailedTransaction() { + purchaseHandler.onPurchaseStateChange(BillingConstants.PurchaseState.CANCELED, BillingConstants.TASKS_DONATION_ITEM_ID); + + assertFalse(purchaseHandler.userDonated()); + } + + @Test + public void ignoreOldItems() { + purchaseHandler.onPurchaseStateChange(BillingConstants.PurchaseState.PURCHASED, "some old purchase"); + + assertFalse(purchaseHandler.userDonated()); + } + + @Test + public void oldItemsDontReplaceLatest() { + purchaseHandler.onPurchaseStateChange(BillingConstants.PurchaseState.PURCHASED, BillingConstants.TASKS_DONATION_ITEM_ID); + purchaseHandler.onPurchaseStateChange(BillingConstants.PurchaseState.PURCHASED, "some old purchase"); + + assertTrue(purchaseHandler.userDonated()); + } + + @Test + public void restoredTransactions() { + purchaseHandler.onRestoreTransactionsResponse(BillingConstants.ResponseCode.RESULT_OK); + + assertTrue(purchaseHandler.restoredTransactions()); + } + + @Test + public void restoreTransactionsFailed() { + purchaseHandler.onRestoreTransactionsResponse(BillingConstants.ResponseCode.RESULT_DEVELOPER_ERROR); + + assertFalse(purchaseHandler.restoredTransactions()); + } +}