search

novoda / download-manager

A library that handles long-running downloads, handling the network interactions and retrying downloads automatically after failures
Apache License 2.0
483 stars 63 forks source link

Downloaded notification re-shown #488

Closed Mecharyry closed 5 years ago

Mecharyry commented 5 years ago

 Problem

The DOWNLOADED notification is show again whenever a network recovery mechanism is selected.

When a network recovery mechanism is selected it requeues downloads against the download-manager. If the underlying Map<DownloadBatchId, DownloadBatch> already contains the Batch to queue then it will not add the new Batch. Unfortunately this Batch is the original and it was never updated with notificationSeen=true

Solution

Add a method to the DownloadBatchStatus to update the notificationSeen status. This update will be propagated to the Map<DownloadBatchId, DownloadBatch> solving the issue.

Map<DownloadBatchId, DownloadBatch>`

Someone is bound to ask the question "why not update the map each time"...

If someone were to trigger the network recovery multiple times then we end up queueing multiple downloads against the ExecutorService. Which in itself is not a problem, it only is if you allow each DownloadBatchId to have a different state, which could potentially happen. Let's say we trigger a download, a network error then occurs and then we decide to delete it. Well then the download would finish and then the delete would occur. Instead you supply a single consistent instance for multiple threads, that way when the first terminates, even with it queuing a duplicate it will have a consistent state and just go straight to the end state.

Future work

Dorvaryn commented 5 years ago

Nice one ^^ As you mention there is a fair bit of hidden knowledge here with the cached vs updated values. But that can be worked on as an improvement. As a fix this is :+1:

Mecharyry commented 5 years ago

Thanks @Dorvaryn 😄 I've created a ticket to document this issue in a bit more detail and recommend some changes. https://github.com/novoda/download-manager/issues/489