Merge pull request #53 from button/najm/allow-broken-requests-temp

najmsheikh · web-flow · commit f3e84abacaef · 2020-07-29T12:17:10.000-04:00
Allow broken events requests to be made
diff --git a/button-merchant/src/main/java/com/usebutton/merchant/ButtonApi.java b/button-merchant/src/main/java/com/usebutton/merchant/ButtonApi.java
@@ -40,6 +40,9 @@ interface ButtonApi {
 
     void setApplicationId(String applicationId);
 
+    @Nullable
+    String getApplicationId();
+
     @Nullable
     @WorkerThread
     PostInstallLink getPendingLink(String applicationId, @Nullable String advertisingId,
diff --git a/button-merchant/src/main/java/com/usebutton/merchant/ButtonApiImpl.java b/button-merchant/src/main/java/com/usebutton/merchant/ButtonApiImpl.java
@@ -67,6 +67,12 @@ public void setApplicationId(String applicationId) {
         connectionManager.setApplicationId(applicationId);
     }
 
+    @Nullable
+    @Override
+    public String getApplicationId() {
+        return connectionManager.getApplicationId();
+    }
+
     @Nullable
     @WorkerThread
     @Override
diff --git a/button-merchant/src/main/java/com/usebutton/merchant/ButtonInternalImpl.java b/button-merchant/src/main/java/com/usebutton/merchant/ButtonInternalImpl.java
@@ -39,7 +39,6 @@
 import java.util.Set;
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.regex.Pattern;
 
 /**
  * ButtonInternalImpl class should implement everything needed for {@link ButtonMerchant}
@@ -72,19 +71,16 @@ final class ButtonInternalImpl implements ButtonInternal {
      */
     private final AtomicBoolean hasReceivedDirectDeeplink = new AtomicBoolean();
 
-    private static final Pattern APP_ID_PATTERN = Pattern.compile("^app-[0-9a-zA-Z]+$");
-
     ButtonInternalImpl(Executor executor) {
         this.attributionTokenListeners = new ArrayList<>();
         this.executor = executor;
     }
 
     public void configure(ButtonRepository buttonRepository, String applicationId) {
-        if (!APP_ID_PATTERN.matcher(applicationId).matches()) {
+        if (!ButtonUtil.isApplicationIdValid(applicationId)) {
             Log.e(TAG, "Application ID [" + applicationId + "] is not valid. "
                     + "You can find your Application ID in the dashboard by logging in at"
                     + " https://app.usebutton.com/");
-            return;
         }
 
         buttonRepository.setApplicationId(applicationId);
diff --git a/button-merchant/src/main/java/com/usebutton/merchant/ButtonMerchant.java b/button-merchant/src/main/java/com/usebutton/merchant/ButtonMerchant.java
@@ -51,7 +51,7 @@ private ButtonMerchant() {
     static ButtonInternal buttonInternal = new ButtonInternalImpl(executor);
 
     private static ExecutorService executorService = Executors.newSingleThreadExecutor();
-    private static final String BASE_URL = "https://mobileapi.usebutton.com";
+    static final String BASE_URL = "https://mobileapi.usebutton.com";
     static final String FMT_BASE_URL_APP_ID = "https://%s.mobileapi.usebutton.com";
 
     /**
diff --git a/button-merchant/src/main/java/com/usebutton/merchant/ButtonRepositoryImpl.java b/button-merchant/src/main/java/com/usebutton/merchant/ButtonRepositoryImpl.java
@@ -48,9 +48,8 @@ final class ButtonRepositoryImpl implements ButtonRepository {
     private final PersistenceManager persistenceManager;
     private final ExecutorService executorService;
 
-    private String applicationId;
-
     private static ButtonRepository buttonRepository;
+    private boolean isConfigured;
     private List<Task<?>> pendingTasks = new CopyOnWriteArrayList<>();
 
     static ButtonRepository getInstance(ButtonApi buttonApi, PersistenceManager persistenceManager,
@@ -73,7 +72,7 @@ static ButtonRepository getInstance(ButtonApi buttonApi, PersistenceManager pers
 
     @Override
     public void setApplicationId(String applicationId) {
-        this.applicationId = applicationId;
+        isConfigured = true;
         buttonApi.setApplicationId(applicationId);
 
         for (Task<?> task : pendingTasks) {
@@ -85,7 +84,7 @@ public void setApplicationId(String applicationId) {
     @Nullable
     @Override
     public String getApplicationId() {
-        return applicationId;
+        return buttonApi.getApplicationId();
     }
 
     @Override
@@ -157,7 +156,7 @@ public void onTaskError(Throwable throwable) {
      * @param task the task to submit
      */
     private void invokeIfConfigured(Task<?> task) {
-        if (getApplicationId() != null) {
+        if (isConfigured) {
             executorService.submit(task);
         } else {
             Log.d(TAG, "Application ID unavailable! Queueing Task.");
diff --git a/button-merchant/src/main/java/com/usebutton/merchant/ButtonUtil.java b/button-merchant/src/main/java/com/usebutton/merchant/ButtonUtil.java
@@ -61,9 +61,10 @@ final class ButtonUtil {
 
     private static final Pattern EMAIL_REGEX_PATTERN = Pattern.compile(EMAIL_REGEX);
 
+    private static final Pattern APP_ID_PATTERN = Pattern.compile("^app-[0-9a-zA-Z]+$");
+
     /**
      * @param date date to be formatted
-     *
      * @return date formatted in ISO_8601 format
      */
     public static String formatDate(Date date) {
@@ -92,6 +93,10 @@ public static boolean isValidEmail(String email) {
         return EMAIL_REGEX_PATTERN.matcher(email).matches();
     }
 
+    public static boolean isApplicationIdValid(@Nullable String applicationId) {
+        return applicationId != null && APP_ID_PATTERN.matcher(applicationId).matches();
+    }
+
     private static String encodeHex(byte[] digest) {
         StringBuilder sb = new StringBuilder();
         for (int i = 0; i < digest.length; i++) {
diff --git a/button-merchant/src/main/java/com/usebutton/merchant/ConnectionManager.java b/button-merchant/src/main/java/com/usebutton/merchant/ConnectionManager.java
@@ -25,6 +25,8 @@
 
 package com.usebutton.merchant;
 
+import android.support.annotation.Nullable;
+
 import com.usebutton.merchant.exception.ButtonNetworkException;
 
 /**
@@ -34,5 +36,8 @@ interface ConnectionManager {
 
     void setApplicationId(String applicationId);
 
+    @Nullable
+    String getApplicationId();
+
     NetworkResponse executeRequest(ApiRequest request) throws ButtonNetworkException;
 }
diff --git a/button-merchant/src/main/java/com/usebutton/merchant/ConnectionManagerImpl.java b/button-merchant/src/main/java/com/usebutton/merchant/ConnectionManagerImpl.java
@@ -90,10 +90,21 @@ static ConnectionManager getInstance(String baseUrl, String userAgent,
 
     @Override
     public void setApplicationId(String applicationId) {
-        this.applicationId = applicationId;
+        // Only cache the application ID if it is valid
+        // However, we would still let the network requests proceed instead of failing silently
+        // on the client-side.
+        if (ButtonUtil.isApplicationIdValid(applicationId)) {
+            this.applicationId = applicationId;
+
+            // Update the endpoint to the App ID flavor
+            baseUrl = String.format(ButtonMerchant.FMT_BASE_URL_APP_ID, applicationId);
+        }
+    }
 
-        // Update the endpoint to the App ID flavor
-        baseUrl = String.format(ButtonMerchant.FMT_BASE_URL_APP_ID, applicationId);
+    @Nullable
+    @Override
+    public String getApplicationId() {
+        return applicationId;
     }
 
     @Override
diff --git a/button-merchant/src/test/java/com/usebutton/merchant/ButtonInternalImplTest.java b/button-merchant/src/test/java/com/usebutton/merchant/ButtonInternalImplTest.java
@@ -50,7 +50,6 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
 public class ButtonInternalImplTest {
@@ -73,15 +72,6 @@ public void configure_saveApplicationIdInMemory() {
         verify(buttonRepository).setApplicationId("app-abcdef1234567890");
     }
 
-    @Test
-    public void configure_invalidAppId_shouldNotConfigure() {
-        ButtonRepository buttonRepository = mock(ButtonRepository.class);
-
-        buttonInternal.configure(buttonRepository, "invalid_application_id");
-
-        verifyZeroInteractions(buttonRepository);
-    }
-
     @Test
     public void getApplicationId_retrieveFromRepository() {
         ButtonRepository buttonRepository = mock(ButtonRepository.class);
diff --git a/button-merchant/src/test/java/com/usebutton/merchant/ButtonRepositoryImplTest.java b/button-merchant/src/test/java/com/usebutton/merchant/ButtonRepositoryImplTest.java
@@ -40,7 +40,6 @@
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
 public class ButtonRepositoryImplTest {
@@ -62,17 +61,10 @@ public void setUp() {
         buttonRepository = new ButtonRepositoryImpl(buttonApi, persistenceManager, executorService);
     }
 
-    @Test
-    public void setApplicationId_cacheInMemory() {
-        buttonRepository.setApplicationId("valid_application_id");
-        assertEquals("valid_application_id", buttonRepository.getApplicationId());
-        verifyZeroInteractions(persistenceManager);
-    }
-
     @Test
     public void setApplicationId_provideToButtonApi() {
-        buttonRepository.setApplicationId("valid_application_id");
-        verify(buttonApi).setApplicationId("valid_application_id");
+        buttonRepository.setApplicationId("invalid_application_id");
+        verify(buttonApi).setApplicationId("invalid_application_id");
     }
 
     @Test
@@ -84,7 +76,7 @@ public void setApplicationId_submitsPendingTasks() {
         buttonRepository.reportEvent(mock(DeviceManager.class), mock(Features.class),
                 mock(Event.class));
 
-        buttonRepository.setApplicationId("valid_application_id");
+        buttonRepository.setApplicationId("invalid_application_id");
 
         verify(executorService, times(3)).submit(any(EventReportingTask.class));
     }
@@ -98,21 +90,15 @@ public void setApplicationId_clearsPendingTasksAfterSubmission() {
         buttonRepository.reportEvent(mock(DeviceManager.class), mock(Features.class),
                 mock(Event.class));
 
-        buttonRepository.setApplicationId("valid_application_id");
-        buttonRepository.setApplicationId("valid_application_id");
-        buttonRepository.setApplicationId("valid_application_id");
-        buttonRepository.setApplicationId("valid_application_id");
-        buttonRepository.setApplicationId("valid_application_id");
+        buttonRepository.setApplicationId("invalid_application_id");
+        buttonRepository.setApplicationId("invalid_application_id");
+        buttonRepository.setApplicationId("invalid_application_id");
+        buttonRepository.setApplicationId("invalid_application_id");
+        buttonRepository.setApplicationId("invalid_application_id");
 
         verify(executorService, times(3)).submit(any(EventReportingTask.class));
     }
 
-    @Test
-    public void getApplicationId_returnValidAppId() {
-        buttonRepository.setApplicationId("valid_application_id");
-        assertEquals("valid_application_id", buttonRepository.getApplicationId());
-    }
-
     @Test
     public void setSourceToken_persistToPersistenceManager() {
         buttonRepository.setSourceToken("valid_source_token");
@@ -166,7 +152,7 @@ public void postOrder_executeTask() {
 
     @Test
     public void reportEvent_configured_executeTask() {
-        buttonRepository.setApplicationId("valid_application_id");
+        buttonRepository.setApplicationId("invalid_application_id");
         buttonRepository.reportEvent(mock(DeviceManager.class), mock(Features.class),
                 mock(Event.class));
 
@@ -179,7 +165,7 @@ public void reportEvent_unConfigured_queueTaskAndExecuteWhenConfigured() {
                 mock(Event.class));
 
         verify(executorService, never()).submit(any(EventReportingTask.class));
-        buttonRepository.setApplicationId("valid_application_id");
+        buttonRepository.setApplicationId("invalid_application_id");
 
         verify(executorService).submit(any(EventReportingTask.class));
     }
diff --git a/button-merchant/src/test/java/com/usebutton/merchant/ConnectionManagerImplTest.java b/button-merchant/src/test/java/com/usebutton/merchant/ConnectionManagerImplTest.java
@@ -44,9 +44,11 @@
 import java.util.Random;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
 public class ConnectionManagerImplTest {
@@ -75,16 +77,45 @@ public void tearDown() throws Exception {
         server.close();
     }
 
+    @Test
+    public void setApplicationId_cacheInMemory() {
+        connectionManager.setApplicationId("app-abcdef1234567890");
+        assertEquals("app-abcdef1234567890", connectionManager.getApplicationId());
+        verifyZeroInteractions(persistenceManager);
+    }
+
     @Test
     public void setApplicationId_shouldUpdateEndpoint() {
-        connectionManager = new ConnectionManagerImpl(url, VALID_UA, persistenceManager);
+        connectionManager = new ConnectionManagerImpl(ButtonMerchant.BASE_URL, VALID_UA,
+                persistenceManager);
 
-        connectionManager.setApplicationId("valid_app_id");
+        connectionManager.setApplicationId("app-abcdef1234567890");
 
-        String expectedUrl = String.format(ButtonMerchant.FMT_BASE_URL_APP_ID, "valid_app_id");
+        String expectedUrl =
+                String.format(ButtonMerchant.FMT_BASE_URL_APP_ID, "app-abcdef1234567890");
         assertEquals(expectedUrl, ((ConnectionManagerImpl) connectionManager).baseUrl);
     }
 
+    @Test
+    public void setApplicationId_invalidAppId_shouldNotCacheOrUpdateEndpoint() {
+        connectionManager.setApplicationId("invalid_app_id");
+
+        assertNull(connectionManager.getApplicationId());
+        assertEquals(url, ((ConnectionManagerImpl) connectionManager).baseUrl);
+    }
+
+    @Test
+    public void getApplicationId_validAppId_returnValidAppId() {
+        connectionManager.setApplicationId("app-abcdef1234567890");
+        assertEquals("app-abcdef1234567890", connectionManager.getApplicationId());
+    }
+
+    @Test
+    public void getApplicationId_invalidAppId_returnNull() {
+        connectionManager.setApplicationId("invalid_application_id");
+        assertNull(connectionManager.getApplicationId());
+    }
+
     @Test(expected = ButtonNetworkException.class)
     public void executeRequest_invalidUrl_shouldThrowError() throws Exception {
         String url = "invalid_link";
@@ -266,7 +297,7 @@ public void executeRequest_shouldIncludeSessionId() throws Exception {
 
     @Test
     public void executeRequest_shouldIncludeApplicationId() throws Exception {
-        String applicationId = "valid_application_id";
+        String applicationId = "app-abcdef1234567890";
         connectionManager.setApplicationId(applicationId);
         ((ConnectionManagerImpl) connectionManager).baseUrl = url;
         server.enqueue(new MockResponse().setResponseCode(200).setBody("{}"));
