Persistent delete notification (#508)

Persistent delete notification
novoda · Jul 26, 2019 · 6b55105 · 6b55105
2 parents a35a1c3 + c9ca8a3
commit 6b55105
Show file tree

Hide file tree

Showing 6 changed files with 13 additions and 10 deletions.
diff --git a/demo-simple/build.gradle b/demo-simple/build.gradle
@@ -36,7 +36,7 @@ dependencies {
     implementation 'com.facebook.stetho:stetho:1.5.1'
     implementation 'com.android.support:appcompat-v7:28.0.0'
     implementation 'com.android.support.constraint:constraint-layout:1.1.3'
-    implementation 'com.squareup.okhttp3:okhttp:3.12.1'
+    implementation 'com.squareup.okhttp3:okhttp:3.14.2'
     implementation project(path: ':library')
     testImplementation 'org.mockito:mockito-core:2.28.2'
     testImplementation 'com.google.truth:truth:0.44'

diff --git a/library/build.gradle b/library/build.gradle
@@ -70,7 +70,7 @@ dependencies {
     annotationProcessor 'android.arch.persistence.room:compiler:1.1.1'
     implementation 'com.novoda:merlin:1.2.1'
     implementation 'com.facebook.stetho:stetho:1.5.1'
-    implementation 'com.squareup.okhttp3:okhttp:3.12.1'
+    implementation 'com.squareup.okhttp3:okhttp:3.14.2'
     implementation('android.arch.persistence.room:runtime:1.1.1') {
         exclude group: 'com.android.support', module: 'support-v4'
         exclude group: 'android.arch.core', module: 'runtime'

diff --git a/library/src/main/java/com/novoda/downloadmanager/LiteDownloadManager.java b/library/src/main/java/com/novoda/downloadmanager/LiteDownloadManager.java
@@ -120,7 +120,6 @@ public void delete(DownloadBatchId downloadBatchId) {
         }
 
         downloadBatch.delete();
-        downloadBatchMap.remove(downloadBatchId);
     }
 
     @Override

diff --git a/library/src/main/java/com/novoda/downloadmanager/LiteDownloadManagerDownloader.java b/library/src/main/java/com/novoda/downloadmanager/LiteDownloadManagerDownloader.java
@@ -119,10 +119,6 @@ private DownloadBatchStatusCallback downloadBatchCallback(Map<DownloadBatchId, D
             }
 
             DownloadBatchId downloadBatchId = downloadBatchStatus.getDownloadBatchId();
-            if (downloadBatchStatus.status() == DELETED) {
-                Logger.v("batch " + downloadBatchId.rawId() + " is finally deleted, removing it from the map");
-                downloadBatchMap.remove(downloadBatchId);
-            }
 
             callbackHandler.post(() -> {
                 synchronized (waitForDownloadBatchStatusCallback) {
@@ -133,6 +129,11 @@ private DownloadBatchStatusCallback downloadBatchCallback(Map<DownloadBatchId, D
                     DownloadBatch downloadBatch = downloadBatchMap.get(downloadBatchId);
                     if (downloadBatch != null) {
                         notificationDispatcher.updateNotification(downloadBatch.status());
+
+                        if (downloadBatch.status().status() == DELETED) {
+                            Logger.v("batch " + downloadBatchId.rawId() + " is finally deleted, removing it from the map");
+                            downloadBatchMap.remove(downloadBatchId);
+                        }
                     }
                 }
             });

diff --git a/...c/test/java/com/novoda/downloadmanager/DownloadBatchStatusNotificationDispatcherTest.java b/...c/test/java/com/novoda/downloadmanager/DownloadBatchStatusNotificationDispatcherTest.java
@@ -58,5 +58,4 @@ public void setsDownloadServiceOnNotificationDispatcher() {
 
         verify(notificationDispatcher).setService(downloadService);
     }
-
 }
diff --git a/library/src/test/java/com/novoda/downloadmanager/LiteDownloadManagerTest.java b/library/src/test/java/com/novoda/downloadmanager/LiteDownloadManagerTest.java
@@ -200,11 +200,15 @@ public void deletesBatch() {
             verify(downloadBatch).delete();
         }
 
+        /**
+         * The removal from the map occurs in {@link LiteDownloadManagerDownloader} after the notification has been sent
+         * to avoid race conditions between the map removal and the executor attempting to send notifications.
+         */
         @Test
-        public void removesBatchFromMap_whenDeletingBatch() {
+        public void doesNotRemoveBatchFromMap_whenDeletingBatch() {
             liteDownloadManager.delete(DOWNLOAD_BATCH_ID);
 
-            assertThat(downloadingBatches).doesNotContainKey(DOWNLOAD_BATCH_ID);
+            assertThat(downloadingBatches).containsKey(DOWNLOAD_BATCH_ID);
         }
 
         @Test
-Original file line number
+Diff line change
@@ Expand Up / @@ -120,7 +120,6 @@ public void delete(DownloadBatchId downloadBatchId) { @@
             }
             downloadBatch.delete();
-            downloadBatchMap.remove(downloadBatchId);
         }
         @Override
@@ Expand Down @@
Original file line number	Diff line number	Diff line change
Expand Up		@@ -58,5 +58,4 @@ public void setsDownloadServiceOnNotificationDispatcher() {

		verify(notificationDispatcher).setService(downloadService);
		}

		}