Update billing

* Add unit tests
* Dont start service if user already donated - #53

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
 }

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";
 

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<Long, BillingRequest> 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()) {

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);
             }
         });
     }

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);
         }
     }
 }

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);
+    }
 }

astrid/src/test/java/org/tasks/SampleTest.java

@@ -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() {
-    }
-}

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();
+    }
+}

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());
+    }
+}