From 4682f376e305c97f26a197ee7afce0a1116c6665 Mon Sep 17 00:00:00 2001 From: Michael Shafrir <45020849+mshafrir-stripe@users.noreply.github.com> Date: Wed, 12 Jun 2019 14:26:43 -0400 Subject: [PATCH] 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. --- .../com/stripe/android/RequestExecutor.java | 6 ++++- .../com/stripe/android/StripeApiHandler.java | 11 ++++++++- .../stripe/android/StripeNetworkUtils.java | 10 ++++---- .../java/com/stripe/android/StripeUid.java | 16 +++++++++++++ .../java/com/stripe/android/Supplier.java | 7 ++++++ .../stripe/android/TelemetryClientUtil.java | 10 +++++--- .../java/com/stripe/android/UidProvider.java | 20 ---------------- .../java/com/stripe/android/UidSupplier.java | 21 +++++++++++++++++ .../com/stripe/android/ApiRequestTest.java | 16 +++++++------ .../com/stripe/android/FakeUidSupplier.java | 23 +++++++++++++++++++ .../android/StripeNetworkUtilsTest.java | 19 +++++---------- .../java/com/stripe/android/StripeTest.java | 20 ++++++++++++---- 12 files changed, 124 insertions(+), 55 deletions(-) create mode 100644 stripe/src/main/java/com/stripe/android/StripeUid.java create mode 100644 stripe/src/main/java/com/stripe/android/Supplier.java delete mode 100644 stripe/src/main/java/com/stripe/android/UidProvider.java create mode 100644 stripe/src/main/java/com/stripe/android/UidSupplier.java create mode 100644 stripe/src/test/java/com/stripe/android/FakeUidSupplier.java diff --git a/stripe/src/main/java/com/stripe/android/RequestExecutor.java b/stripe/src/main/java/com/stripe/android/RequestExecutor.java index 46d6c6f2d9a..74718cf4871 100644 --- 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 index 6f98453891f..d4f749573f2 100644 --- 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 index 405b26dd874..8f110e0db9a 100644 --- 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 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 uidSupplier) { mPackageName = packageName; - mUidProvider = uidProvider; + mUidSupplier = uidSupplier; } /** @@ -168,7 +168,7 @@ void addUidParamsToPaymentIntent(@NonNull Map params) { @NonNull Map 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 new file mode 100644 index 00000000000..ef07a324e42 --- /dev/null +++ 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 new file mode 100644 index 00000000000..406906ee8dc --- /dev/null +++ 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 { + @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 index 9f0b2faeabc..d20384abecd 100644 --- 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 mUidSupplier; TelemetryClientUtil(@NonNull Context context) { + this(context, new UidSupplier(context)); + } + + TelemetryClientUtil(@NonNull Context context, @NonNull Supplier 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 deleted file mode 100644 index 04d5e60d441..00000000000 --- a/stripe/src/main/java/com/stripe/android/UidProvider.java +++ /dev/null @@ -1,20 +0,0 @@ -package com.stripe.android; - -import android.content.Context; -import android.provider.Settings; -import android.support.annotation.NonNull; - -class UidProvider { - @NonNull private final Context mContext; - - UidProvider(@NonNull Context context) { - mContext = context.getApplicationContext(); - } - - @SuppressWarnings("HardwareIds") - @NonNull - String get() { - return Settings.Secure.getString(mContext.getContentResolver(), - Settings.Secure.ANDROID_ID); - } -} diff --git a/stripe/src/main/java/com/stripe/android/UidSupplier.java b/stripe/src/main/java/com/stripe/android/UidSupplier.java new file mode 100644 index 00000000000..12f6142da51 --- /dev/null +++ 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 { + @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 index c7cd73fd034..4af4e955ed3 100644 --- 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 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 new file mode 100644 index 00000000000..842940e9205 --- /dev/null +++ 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 { + @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 index 8b5adef9195..bc5ab01ad9d 100644 --- 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 uidParams = - new StripeNetworkUtils("com.example.main", mUidProvider) - .createUidParams(); + final Map 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 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 getBankAccountTokenParamData(@NonNull BankAccount ba assertNotNull(bankAccountTokenParams); return (Map) 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 index 67d99cd93e7..36e2cdc37d3 100644 --- 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())); } }