Handle null response body (#1104)

Also create `Supplier` interface to be able to use a fake `UidSupplier` in tests to ensure that a non-null id is returned.
stripe · Jun 12, 2019 · 4682f37 · 4682f37
1 parent 3db7504
commit 4682f37
Show file tree

Hide file tree

Showing 12 changed files with 124 additions and 55 deletions.
diff --git a/stripe/src/main/java/com/stripe/android/RequestExecutor.java b/stripe/src/main/java/com/stripe/android/RequestExecutor.java
@@ -65,8 +65,12 @@ StripeResponse execute(@NonNull StripeRequest request)
     }
 
     @Nullable
-    private String getResponseBody(@NonNull InputStream responseStream)
+    private String getResponseBody(@Nullable InputStream responseStream)
             throws IOException {
+        if (responseStream == null) {
+            return null;
+        }
+
         //\A is the beginning of
         // the stream boundary
         final Scanner scanner = new Scanner(responseStream, CHARSET).useDelimiter("\\A");

diff --git a/stripe/src/main/java/com/stripe/android/StripeApiHandler.java b/stripe/src/main/java/com/stripe/android/StripeApiHandler.java
@@ -60,10 +60,19 @@ class StripeApiHandler {
                      @NonNull RequestExecutor requestExecutor,
                      boolean shouldLogRequest,
                      @Nullable AppInfo appInfo) {
+        this(context, requestExecutor, shouldLogRequest, appInfo, new TelemetryClientUtil(context));
+    }
+
+    @VisibleForTesting
+    StripeApiHandler(@NonNull Context context,
+                     @NonNull RequestExecutor requestExecutor,
+                     boolean shouldLogRequest,
+                     @Nullable AppInfo appInfo,
+                     @NonNull TelemetryClientUtil telemetryClientUtil) {
         mRequestExecutor = requestExecutor;
         mShouldLogRequest = shouldLogRequest;
         mLoggingUtils = new LoggingUtils(context);
-        mTelemetryClientUtil = new TelemetryClientUtil(context);
+        mTelemetryClientUtil = telemetryClientUtil;
         mNetworkUtils = new StripeNetworkUtils(context);
         mAppInfo = appInfo;
     }

diff --git a/stripe/src/main/java/com/stripe/android/StripeNetworkUtils.java b/stripe/src/main/java/com/stripe/android/StripeNetworkUtils.java
@@ -24,16 +24,16 @@ public class StripeNetworkUtils {
     private static final String FIELD_GUID = "guid";
 
     @NonNull private final String mPackageName;
-    @NonNull private final UidProvider mUidProvider;
+    @NonNull private final Supplier<StripeUid> mUidSupplier;
 
     StripeNetworkUtils(@NonNull Context context) {
-        this(context.getPackageName(), new UidProvider(context));
+        this(context.getPackageName(), new UidSupplier(context));
     }
 
     @VisibleForTesting
-    StripeNetworkUtils(@NonNull String packageName, @NonNull UidProvider uidProvider) {
+    StripeNetworkUtils(@NonNull String packageName, @NonNull Supplier<StripeUid> uidSupplier) {
         mPackageName = packageName;
-        mUidProvider = uidProvider;
+        mUidSupplier = uidSupplier;
     }
 
     /**
@@ -168,7 +168,7 @@ void addUidParamsToPaymentIntent(@NonNull Map<String, Object> params) {
 
     @NonNull
     Map<String, String> createUidParams() {
-        final String guid = mUidProvider.get();
+        final String guid = mUidSupplier.get().value;
         if (StripeTextUtils.isBlank(guid)) {
             return new HashMap<>();
         }

diff --git a/stripe/src/main/java/com/stripe/android/StripeUid.java b/stripe/src/main/java/com/stripe/android/StripeUid.java
@@ -0,0 +1,16 @@
+package com.stripe.android;
+
+import android.support.annotation.NonNull;
+
+final class StripeUid {
+    @NonNull final String value;
+
+    @NonNull
+    static StripeUid create(@NonNull String uid) {
+        return new StripeUid(uid);
+    }
+
+    private StripeUid(@NonNull String value) {
+        this.value = value;
+    }
+}
diff --git a/stripe/src/main/java/com/stripe/android/Supplier.java b/stripe/src/main/java/com/stripe/android/Supplier.java
@@ -0,0 +1,7 @@
+package com.stripe.android;
+
+import android.support.annotation.NonNull;
+
+interface Supplier<T> {
+    @NonNull T get();
+}
diff --git a/stripe/src/main/java/com/stripe/android/TelemetryClientUtil.java b/stripe/src/main/java/com/stripe/android/TelemetryClientUtil.java
@@ -17,11 +17,15 @@
 class TelemetryClientUtil {
 
     @NonNull private final Context mContext;
-    @NonNull private final UidProvider mUidProvider;
+    @NonNull private final Supplier<StripeUid> mUidSupplier;
 
     TelemetryClientUtil(@NonNull Context context) {
+        this(context, new UidSupplier(context));
+    }
+
+    TelemetryClientUtil(@NonNull Context context, @NonNull Supplier<StripeUid> uidSupplier) {
         mContext = context.getApplicationContext();
-        mUidProvider = new UidProvider(context);
+        mUidSupplier = uidSupplier;
     }
 
     @NonNull
@@ -111,7 +115,7 @@ private static String getAndroidVersionString() {
 
     @NonNull
     String getHashedId() {
-        final String id = mUidProvider.get();
+        final String id = mUidSupplier.get().value;
         if (StripeTextUtils.isBlank(id)) {
             return "";
         }

diff --git a/stripe/src/main/java/com/stripe/android/UidProvider.java b/stripe/src/main/java/com/stripe/android/UidProvider.java
diff --git a/stripe/src/main/java/com/stripe/android/UidSupplier.java b/stripe/src/main/java/com/stripe/android/UidSupplier.java
@@ -0,0 +1,21 @@
+package com.stripe.android;
+
+import android.content.ContentResolver;
+import android.content.Context;
+import android.provider.Settings;
+import android.support.annotation.NonNull;
+
+final class UidSupplier implements Supplier<StripeUid> {
+    @NonNull private final ContentResolver mContentResolver;
+
+    UidSupplier(@NonNull Context context) {
+        mContentResolver = context.getApplicationContext().getContentResolver();
+    }
+
+    @SuppressWarnings("HardwareIds")
+    @NonNull
+    public StripeUid get() {
+        return StripeUid.create(
+                Settings.Secure.getString(mContentResolver, Settings.Secure.ANDROID_ID));
+    }
+}
diff --git a/stripe/src/test/java/com/stripe/android/ApiRequestTest.java b/stripe/src/test/java/com/stripe/android/ApiRequestTest.java
@@ -1,6 +1,7 @@
 package com.stripe.android;
 
 import android.os.Build;
+import android.support.annotation.NonNull;
 
 import com.stripe.android.exception.InvalidRequestException;
 import com.stripe.android.model.CardFixtures;
@@ -10,7 +11,6 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.RobolectricTestRunner;
 
@@ -24,16 +24,19 @@
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.when;
 
 @RunWith(RobolectricTestRunner.class)
 public class ApiRequestTest {
-    @Mock private UidProvider mUidProvider;
+
+    @NonNull
+    private final StripeNetworkUtils mNetworkUtils = new StripeNetworkUtils(
+            "com.example.app",
+            new FakeUidSupplier("abc123")
+    );
 
     @Before
     public void setup() {
         MockitoAnnotations.initMocks(this);
-        when(mUidProvider.get()).thenReturn("abc-def-123-456");
     }
 
     @Test
@@ -100,13 +103,12 @@ public void getHeaders_correctlyAddsExpectedAdditionalParameters() {
     public void createQuery_withCardData_createsProperQueryString()
             throws UnsupportedEncodingException, InvalidRequestException {
         final Map<String, Object> cardMap =
-                new StripeNetworkUtils("com.example.app", mUidProvider)
-                        .createCardTokenParams(CardFixtures.MINIMUM_CARD);
+                mNetworkUtils.createCardTokenParams(CardFixtures.MINIMUM_CARD);
         final String query = ApiRequest.createGet(StripeApiHandler.getSourcesUrl(), cardMap,
                 ApiRequest.Options.create(ApiKeyFixtures.DEFAULT_PUBLISHABLE_KEY), null)
                 .createQuery();
 
-        final String expectedValue = "muid=E8B53128C63018F96179F2AC10643BF32B8F707D&product_usage=&guid=A9709C2E70D28D99D345DC09B1F5EBD817DDCDAE&card%5Bnumber%5D=4242424242424242&card%5Bcvc%5D=123&card%5Bexp_month%5D=1&card%5Bexp_year%5D=2050";
+        final String expectedValue = "muid=BF3BF4D775100923AAAFA82884FB759001162E28&product_usage=&guid=6367C48DD193D56EA7B0BAAD25B19455E529F5EE&card%5Bnumber%5D=4242424242424242&card%5Bcvc%5D=123&card%5Bexp_month%5D=1&card%5Bexp_year%5D=2050";
         assertEquals(expectedValue, query);
     }
 

diff --git a/stripe/src/test/java/com/stripe/android/FakeUidSupplier.java b/stripe/src/test/java/com/stripe/android/FakeUidSupplier.java
@@ -0,0 +1,23 @@
+package com.stripe.android;
+
+import android.support.annotation.NonNull;
+
+import java.util.UUID;
+
+final class FakeUidSupplier implements Supplier<StripeUid> {
+    @NonNull private final String mValue;
+
+    FakeUidSupplier() {
+        this(UUID.randomUUID().toString());
+    }
+
+    FakeUidSupplier(@NonNull String mValue) {
+        this.mValue = mValue;
+    }
+
+    @NonNull
+    @Override
+    public StripeUid get() {
+        return StripeUid.create(mValue);
+    }
+}
diff --git a/stripe/src/test/java/com/stripe/android/StripeNetworkUtilsTest.java b/stripe/src/test/java/com/stripe/android/StripeNetworkUtilsTest.java
@@ -14,7 +14,6 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.RobolectricTestRunner;
 
@@ -26,7 +25,6 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.when;
 
 /**
  * Test class for {@link StripeNetworkUtils}
@@ -49,7 +47,8 @@ public class StripeNetworkUtilsTest {
     private static final String BANK_ROUTING_NUMBER = "110000000";
     private static final String BANK_ACCOUNT_HOLDER_NAME = "Lily Thomas";
 
-    @Mock private UidProvider mUidProvider;
+    @NonNull private final StripeNetworkUtils mNetworkUtils =
+            new StripeNetworkUtils("com.example.app", new FakeUidSupplier());
 
     @Before
     public void setup() {
@@ -178,10 +177,7 @@ public void removeNullAndEmptyParams_removesNestedEmptyParams() {
 
     @Test
     public void addUidParams_addsParams() {
-        when(mUidProvider.get()).thenReturn("abc123");
-        final Map<String, String> uidParams =
-                new StripeNetworkUtils("com.example.main", mUidProvider)
-                        .createUidParams();
+        final Map<String, String> uidParams = mNetworkUtils.createUidParams();
         assertEquals(2, uidParams.size());
         assertTrue(uidParams.containsKey("muid"));
         assertTrue(uidParams.containsKey("guid"));
@@ -193,9 +189,7 @@ public void addUidParamsToPaymentIntent_withSource_addsParamsAtRightLevel() {
         final Map<String, Object> sourceDataMap = new HashMap<>();
         existingMap.put(PaymentIntentParams.API_PARAM_SOURCE_DATA, sourceDataMap);
 
-        when(mUidProvider.get()).thenReturn("abc123");
-        new StripeNetworkUtils("abc123", mUidProvider)
-                .addUidParamsToPaymentIntent(existingMap);
+        mNetworkUtils.addUidParamsToPaymentIntent(existingMap);
         assertEquals(1, existingMap.size());
         assertTrue(sourceDataMap.containsKey("muid"));
         assertTrue(sourceDataMap.containsKey("guid"));
@@ -210,9 +204,7 @@ public void addUidParamsToPaymentIntent_withPaymentMethod_addsParamsAtRightLevel
                 .toMap();
         existingMap.put(PaymentIntentParams.API_PARAM_PAYMENT_METHOD_DATA, paymentMethodData);
 
-        when(mUidProvider.get()).thenReturn("abc123");
-        new StripeNetworkUtils("abc123", mUidProvider)
-                .addUidParamsToPaymentIntent(existingMap);
+        mNetworkUtils.addUidParamsToPaymentIntent(existingMap);
         assertEquals(1, existingMap.size());
         assertTrue(paymentMethodData.containsKey("muid"));
         assertTrue(paymentMethodData.containsKey("guid"));
@@ -237,4 +229,5 @@ private Map<String, Object> getBankAccountTokenParamData(@NonNull BankAccount ba
         assertNotNull(bankAccountTokenParams);
         return (Map<String, Object>) bankAccountTokenParams.get("bank_account");
     }
+
 }
diff --git a/stripe/src/test/java/com/stripe/android/StripeTest.java b/stripe/src/test/java/com/stripe/android/StripeTest.java
@@ -1273,8 +1273,7 @@ private Stripe createNonLoggingStripe() {
 
     @NonNull
     private Stripe createNonLoggingStripe(@NonNull String publishableKey) {
-        final StripeApiHandler apiHandler =
-                new StripeApiHandler(mContext, new RequestExecutor(), false, null);
+        final StripeApiHandler apiHandler = createApiHandler(false);
         return new Stripe(
                 apiHandler,
                 new StripeNetworkUtils(mContext),
@@ -1284,8 +1283,7 @@ private Stripe createNonLoggingStripe(@NonNull String publishableKey) {
 
     @NonNull
     private Stripe createNonLoggingStripe(@NonNull Stripe.TokenCreator tokenCreator) {
-        final StripeApiHandler apiHandler =
-                new StripeApiHandler(mContext, new RequestExecutor(), false, null);
+        final StripeApiHandler apiHandler = createApiHandler(false);
         return new Stripe(
                 apiHandler,
                 new StripeNetworkUtils(mContext),
@@ -1297,6 +1295,18 @@ private Stripe createNonLoggingStripe(@NonNull Stripe.TokenCreator tokenCreator)
 
     @NonNull
     private Stripe createLoggingStripe() {
-        return new Stripe(mContext, LOGGING_PK);
+        final StripeApiHandler apiHandler = createApiHandler(true);
+        return new Stripe(
+                apiHandler,
+                new StripeNetworkUtils(mContext),
+                new PaymentController(mContext, apiHandler),
+                LOGGING_PK
+        );
+    }
+
+    @NonNull
+    private StripeApiHandler createApiHandler(boolean shouldLogRequest) {
+        return new StripeApiHandler(mContext, new RequestExecutor(), shouldLogRequest,
+                null, new TelemetryClientUtil(mContext, new FakeUidSupplier()));
     }
 }