Revision 4

Adding suggested changes from PR reviews

Author: Ayyanchira

iterableapi/src/main/java/com/iterable/iterableapi/HealthMonitor.java

@@ -3,66 +3,40 @@
 public class HealthMonitor implements IterableTaskStorage.IterableDatabaseStatusListeners {
     private static final String TAG = "HealthMonitor";
 
-    private boolean errored = false;
+    private boolean databaseErrored = false;
 
     private IterableTaskStorage iterableTaskStorage;
 
     public HealthMonitor(IterableTaskStorage storage) {
         this.iterableTaskStorage = storage;
-        this.iterableTaskStorage.addDataBaseListener(this);
+        this.iterableTaskStorage.addDatabaseStatusListener(this);
     }
 
     public boolean canSchedule() {
         IterableLogger.d(TAG, "canSchedule");
         try {
-            return !(iterableTaskStorage.numberOfTasks() >= IterableConstants.MAX_OFFLINE_OPERATION);
-        } catch (Exception e) {
+            return !(iterableTaskStorage.getNumberOfTasks() >= IterableConstants.OFFLINE_TASKS_LIMIT);
+        } catch (IllegalStateException e) {
             IterableLogger.e(TAG, e.getLocalizedMessage());
-            errored = true;
+            databaseErrored = true;
         }
         return false;
     }
 
     public boolean canProcess() {
-        IterableLogger.d(TAG, "Health monitor can process: " + !errored);
-        return !errored;
-    }
-
-    public void onScheduleError() {
-        IterableLogger.w(TAG, "onScheduleError");
-    }
-
-    void onNextTaskError() {
-        IterableLogger.w(TAG, "onNextTaskError");
-        onDBError();
-    }
-
-    void onDeleteAllTasksError() {
-        IterableLogger.w(TAG, "onDeleteAllTasksError");
-        onDBError();
-    }
-
-    @Override
-    public void isNotReady() {
-        IterableLogger.v(TAG, "DB Not ready notified to healthMonitor");
-        errored = true;
+        IterableLogger.d(TAG, "Health monitor can process: " + !databaseErrored);
+        return !databaseErrored;
     }
 
     @Override
     public void onDBError() {
-        IterableLogger.v(TAG, "DB Error notified to healthMonitor");
-        errored = true;
-    }
-
-    @Override
-    public void isClosed() {
-        IterableLogger.v(TAG, "DB Closed notified to healthMonitor");
-        errored = true;
+        IterableLogger.e(TAG, "DB Error notified to healthMonitor");
+        databaseErrored = true;
     }
 
     @Override
     public void isReady() {
         IterableLogger.v(TAG, "DB Ready notified to healthMonitor");
-        errored = false;
+        databaseErrored = false;
     }
 }

iterableapi/src/main/java/com/iterable/iterableapi/IterableConstants.java

@@ -236,7 +236,7 @@ public final class IterableConstants {
     public static final String ITERABLE_IN_APP_ACTION_DELETE    = "delete";
 
     //Offline operation
-    public static final long MAX_OFFLINE_OPERATION              = 1000;
+    public static final long OFFLINE_TASKS_LIMIT                = 1000;
 
     // URL schemes
     public static final String URL_SCHEME_ITBL = "itbl://";

iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskRunner.java

@@ -144,7 +144,7 @@ private boolean processTask(@NonNull IterableTask task) {
                 response = IterableRequestTask.executeApiRequest(request);
             } catch (Exception e) {
                 IterableLogger.e(TAG, "Error while processing request task", e);
-                healthMonitor.onNextTaskError();
+                healthMonitor.onDBError();
             }
 
             if (response != null) {

iterableapi/src/main/java/com/iterable/iterableapi/IterableTaskStorage.java

@@ -93,7 +93,7 @@ void addTaskCreatedListener(TaskCreatedListener listener) {
         taskCreatedListeners.add(listener);
     }
 
-    void removeTaskCreatedListener(TaskCreatedListener listener) {
+    void removeDatabaseStatusListener(TaskCreatedListener listener) {
         taskCreatedListeners.remove(listener);
     }
 
@@ -255,9 +255,8 @@ ArrayList<String> getAllTaskIds() {
         return taskIds;
     }
 
-    long numberOfTasks() throws Exception {
+    long getNumberOfTasks() throws IllegalStateException {
         if (!isDatabaseReady()) {
-            notifyDBNotReady();
             throw new IllegalStateException("Database is not ready");
         }
         return DatabaseUtils.queryNumEntries(database, ITERABLE_TASK_TABLE_NAME);
@@ -456,57 +455,34 @@ private boolean updateTaskWithContentValues(String id, ContentValues contentValu
     }
 
     private boolean isDatabaseReady() {
-        if (database == null) {
-            notifyDBNotReady();
-            IterableLogger.e(TAG, "Database not initialized");
+        if (database == null || !database.isOpen()) {
+            notifyDBError();
+            IterableLogger.e(TAG, "Database not initialized or is closed");
             return false;
         }
-
-        if (!database.isOpen()) {
-            notifyDBClosed();
-            IterableLogger.e(TAG, "Database is closed");
-            return false;
-        }
-
         return true;
     }
 
     public interface IterableDatabaseStatusListeners {
-        void isNotReady();
-
         void onDBError();
-
-        void isClosed();
-
         void isReady();
     }
 
     private ArrayList<IterableDatabaseStatusListeners> databaseStatusListeners = new ArrayList<>();
 
-    void addDataBaseListener(IterableDatabaseStatusListeners listener) {
+    void addDatabaseStatusListener(IterableDatabaseStatusListeners listener) {
         if (isDatabaseReady()) {
             listener.isReady();
         } else {
-            listener.isNotReady();
+            listener.onDBError();
         }
         databaseStatusListeners.add(listener);
     }
 
-    void removeTaskCreatedListener(IterableDatabaseStatusListeners listener) {
+    void removeDatabaseStatusListener(IterableDatabaseStatusListeners listener) {
         databaseStatusListeners.remove(listener);
     }
 
-    private void notifyDBClosed() {
-        new Handler(Looper.getMainLooper()).post(new Runnable() {
-            @Override
-            public void run() {
-                for (IterableDatabaseStatusListeners listener : databaseStatusListeners) {
-                    listener.isClosed();
-                }
-            }
-        });
-    }
-
     private void notifyDBError() {
         new Handler(Looper.getMainLooper()).post(new Runnable() {
             @Override
@@ -517,26 +493,4 @@ public void run() {
             }
         });
     }
-
-    private void notifyDBReady() {
-        new Handler(Looper.getMainLooper()).post(new Runnable() {
-            @Override
-            public void run() {
-                for (IterableDatabaseStatusListeners listener : databaseStatusListeners) {
-                    listener.isReady();
-                }
-            }
-        });
-    }
-
-    private void notifyDBNotReady() {
-        new Handler(Looper.getMainLooper()).post(new Runnable() {
-            @Override
-            public void run() {
-                for (IterableDatabaseStatusListeners listener : databaseStatusListeners) {
-                    listener.isNotReady();
-                }
-            }
-        });
-    }
 }

iterableapi/src/main/java/com/iterable/iterableapi/OfflineRequestProcessor.java

@@ -78,7 +78,6 @@ boolean isRequestOfflineCompatible(String baseUrl) {
     }
 }
 
-//Mock TaskScheduler for testing purpose.
 class TaskScheduler implements IterableTaskRunner.TaskCompletedListener {
     static HashMap<String, IterableHelper.SuccessHandler> successCallbackMap = new HashMap<>();
     static HashMap<String, IterableHelper.FailureHandler> failureCallbackMap = new HashMap<>();

iterableapi/src/test/java/com/iterable/iterableapi/HealthMonitorTest.java

@@ -27,14 +27,14 @@ public void setUp() {
     @Test
     public void canScheduleFailWhenMaxCountReached() throws Exception {
         HealthMonitor healthMonitor = new HealthMonitor(mockTaskStorage);
-        when(mockTaskStorage.numberOfTasks()).thenReturn(IterableConstants.MAX_OFFLINE_OPERATION);
+        when(mockTaskStorage.getNumberOfTasks()).thenReturn(IterableConstants.OFFLINE_TASKS_LIMIT);
         assertFalse(healthMonitor.canSchedule());
     }
 
     @Test
     public void canScheduleWhenMaxCountNotReached() throws Exception {
         HealthMonitor healthMonitor = new HealthMonitor(mockTaskStorage);
-        when(mockTaskStorage.numberOfTasks()).thenReturn(IterableConstants.MAX_OFFLINE_OPERATION - 1);
+        when(mockTaskStorage.getNumberOfTasks()).thenReturn(IterableConstants.OFFLINE_TASKS_LIMIT - 1);
         assertTrue(healthMonitor.canSchedule());
     }
 
@@ -48,7 +48,7 @@ public void canProcessReturnTrueIfDBok() throws Exception {
     public void canProcessReturnFalseIfDBError() throws Exception {
         IterableTaskStorage taskStorage = IterableTaskStorage.sharedInstance(getContext());
         HealthMonitor healthMonitor = new HealthMonitor(taskStorage);
-        healthMonitor.onNextTaskError();
+        healthMonitor.onDBError();
         assertFalse(healthMonitor.canProcess());
     }
 }