novoda / download-manager

A library that handles long-running downloads, handling the network interactions and retrying downloads automatically after failures

novoda

This repo contains information that is common to all Novoda's Open Source projects.

download-manager's Issues

Serialised downloads show as `download running` instead of queued

Pull the latest changes from AOSP

It'd be good to pull changes/improvements/bugfixes (if any) from the AOSP from Lollipop or even Android M

Download Manager objects creation

This issue gathers hot spots for object creation while downloading in order lead us places where to focus our efforts

Notifications

With notifications	Without notifications

Battery concerns

Currently setting off a download results in GCs roughly every 700ms and a pretty high CPU usage:

We should be careful about allocation of objects and requeries whenever we're inside updateLocked.

This is on a Moto X (2014) running 5.1

Library still creates verbose logs even if logs are turned off

Won't actually print the logs, but creates the string for them. Results in logs of GC churn.

https://github.com/novoda/download-manager/blob/master/library/src/main/java/com/novoda/downloadmanager/lib/DownloadProvider.java#L866

Subscribe to specific notifications

it'd be nice if you could subscribe to, say, download state changes. So instead of getting all the events (i.e. when a file is being downloaded we get events for every chunk of the file that gets downloading), we'd get only state changes (download paused, download started, download failed, download queued...)

Add PRB

Would be useful to know whether we're reducing the tide of AOSP checkstyle issues or not

First "single" download does not start

Steps to repro:

• Go to pause & resume in extended sample
• Press download button

What happens:

• Download does not start until you queue a second download

What should happen:

• Download should start without requiring another download

Remove all upgrade code

we only have one version of the database table at the moment lets simplify the code we have to maintain

[DownloadProvider.onUpgrade:270] Upgrading downloads database from version 0 to version 109, which will destroy all old data

Rename `ACTION`s in `Constants` to be more specific and meaningful

Raised by @amlcurran in https://github.com/novoda/download-manager/pull/160/files#r41991443

The Action names are too generic at the moment, they should be more specific, and maybe they could be split in many or joined together (OPEN vs. LIST)

Actions are here: https://github.com/novoda/download-manager/blob/57f9aa/library/src/main/java/com/novoda/downloadmanager/lib/Constants.java#L42-L67

Investigate performance

We've noticed a few places with duplicated code (download rule checking in both download thread and queue submitting)

Download rule checks are called on every download table update

SQL queries can be better optimised (sum columns etc)

Consider not exposing cursors

It'd be great to expose value objects not a cursor because then we have type-safety and don't enforce lifecycle constraints on our users (e.g. they need to call Cursor#close())

Download sometimes fails when pausing

This may be the cause of our downloads sometimes failing. Key thing that seems to happen is that it can't delete the file?

07-03 16:25:44.885    3467-4047/com.novoda.secretz D/NoTils﹕ [DownloadManager-UpdateThread][DownloadDeleter.deleteFileIfExists:64] deleteFileIfExists() deleting /data/data/com.novoda.secretz/files/60545.tmp
07-03 16:25:45.778    3467-4051/com.novoda.secretz W/NoTils﹕ [pool-7-thread-1][Log.w:99] Aborting request for download 2: download canceled
    com.novoda.downloadmanager.lib.StopRequestException: download canceled
            at com.novoda.downloadmanager.lib.DownloadThread.checkPausedOrCanceled(DownloadThread.java:541)
            at com.novoda.downloadmanager.lib.DownloadThread.transferData(DownloadThread.java:500)
            at com.novoda.downloadmanager.lib.DownloadThread.transferData(DownloadThread.java:427)
            at com.novoda.downloadmanager.lib.DownloadThread.executeDownload(DownloadThread.java:348)
            at com.novoda.downloadmanager.lib.DownloadThread.runInternal(DownloadThread.java:244)
            at com.novoda.downloadmanager.lib.DownloadThread.run(DownloadThread.java:174)
            at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:422)
            at java.util.concurrent.FutureTask.run(FutureTask.java:237)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
            at java.lang.Thread.run(Thread.java:818)
07-03 16:25:45.779    3467-4051/com.novoda.secretz D/NoTils﹕ [pool-7-thread-1][DownloadThread.cleanupDestination:522] cleanupDestination() deleting /data/data/com.novoda.secretz/files/60545.tmp
07-03 16:25:45.779    3467-4051/com.novoda.secretz E/NoTils﹕ [pool-7-thread-1][DownloadThread.cleanupDestination:525] File not deleted
07-03 16:25:46.028    3467-4051/com.novoda.secretz I/NoTils﹕ [pool-7-thread-1][DownloadThread.runInternal:295] Download 2 finished with status CANCELED

COLUMN_TOTAL_SIZE_BYTES + COLUMN_BYTES_DOWNLOADED_SO_FAR

Hello,
It's normaly that COLUMN_BYTES_DOWNLOADED_SO_FAR and COLUMN_TOTAL_SIZE_BYTES
changed only when download is complated and sometimes the COLUMN_TOTAL_SIZE_BYTES stuck on -1??
This is my code to get the values :

int id = cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_ID));

String title = cursor.getString(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_TITLE));

int status = cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS));

long totalBytes = cursor.getLong(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_TOTAL_SIZE_BYTES));

long currentBytes = cursor.getLong(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR));

String latestUpdate = DateFormat.getInstance().format(new Date(Long.valueOf(cursor.getString(
                   cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP)))));

Queuing no longer works

1. Set-up two downloads
2. Pause the first download - the second one starts downloading
3. Resume the first download - the first download goes to "Queued"
4. Pause the second download

What should happen:
The first download should resume again

What does happen:
It says queued and no more downloads occur

Consider public interface

What classes with @hide should be not visible? Perhaps we should add some sub packages

Notification customisers need to be able to customise everything

In https://github.com/novoda/download-manager/blob/downloading-text/library/src/main/java/com/novoda/downloadmanager/notifications/NotificationDisplayer.java

The notification is still modified after using the customisers, meaning some views in the notification can't be modified, we should address this at some point, specially for large notifications (stacked).

Notification labels can't be localized

String like cancel, download complete, etc are hardcoded

download notification removed then appeared when remove app from recent app list

hi , thx for this great library , i am facing issue that every time when i try to download something , when press home button and remove the app from recent app list , the download in notification removed and reapeared ,in addition , the notification stuck at downloading state and not showing any update on progress bar
thx in advance

Improve Request API

Some of the items are to be mandatory, even though it is a builder.Also the saveToInternalDir could be a more generic save to file

Ensure all resources have a library prefix

We need to prefix with dm__ so that we don't get clashes when people use the lib and also easier to find all resources.

i.e.

ic_action_cancel should be dm__ic_action_cancel

Crash can occur when using addCompletedBatch method

Following crash can occur when:

• You are adding a completed batch to the download manager using DownloadManager#addCompletedBatch
• That batch has a notification visibility of NotificationVisibility.ACTIVE_OR_COMPLETE or NotificationVisibility.ONLY_WHEN_COMPLETE

    java.lang.IndexOutOfBoundsException: Invalid index 0, size is 0
            at java.util.ArrayList.throwIndexOutOfBoundsException(ArrayList.java:255)
            at java.util.ArrayList.get(ArrayList.java:308)
            at com.novoda.downloadmanager.lib.NotificationDisplayer.buildActionIntents(NotificationDisplayer.java:107)
            at com.novoda.downloadmanager.lib.NotificationDisplayer.buildAndShowNotification(NotificationDisplayer.java:58)
            at com.novoda.downloadmanager.lib.DownloadNotifier.updateWith(DownloadNotifier.java:85)
            at com.novoda.downloadmanager.lib.DownloadService.updateUserVisibleNotification(DownloadService.java:429)
            at com.novoda.downloadmanager.lib.DownloadService.updateLocked(DownloadService.java:354)
            at com.novoda.downloadmanager.lib.DownloadService.access$200(DownloadService.java:62)
            at com.novoda.downloadmanager.lib.DownloadService$2.handleMessage(DownloadService.java:267)
            at android.os.Handler.dispatchMessage(Handler.java:98)
            at android.os.Looper.loop(Looper.java:145)
            at android.os.HandlerThread.run(HandlerThread.java:61)

This occurs because the notification displayer requires a batch to have at least one item, but the batch must be created before items can be added to it.

Easiest way around it is to not show a notification if the batch has no downloads, or not add a notification click intent

Notify the Activity when Download completed

Notify the Activity when Download completed
Thanks

Remove system downloads code

Some of the code can only be run by the system app (such as accessing all downloads), so we can't use it. We should remove it for clarity.

https://github.com/novoda/download-manager/blob/master/library/src/main/java/com/novoda/downloadmanager/lib/DownloadManager.java#L348
https://github.com/novoda/download-manager/blob/master/library/src/main/java/com/novoda/downloadmanager/lib/Request.java#L149

Add demo for tar updating feature

#120

Delete button sometimes not reacting

In demo-extend:
Delete buttons seems to not respond to clicks sometimes

Nexus 4, Android 5.1

Media scanning, do we need it?

Should we continue to support mime type insertion, gallery meta data etc?

I feel like this is something the client should do on download completion?

Unsuccessful download notification reappearing after being dismissed

This occurs while another download is active

Integration tests

We're refactoring a lot which is good but we need to have confidence that we aren't breaking anything. It'd be good to have some integration or even automation tests to flag failures early

Notification view intent uses android.app.DownloadManager

Should use our own (is this why we're getting crashes?) this just doesn't do anything when you click the notification

https://github.com/novoda/download-manager/blob/master/library/src/main/java/com/novoda/downloadmanager/lib/OpenHelper.java#L37

downloadManager.pauseBatch(download.getBatchId()); not working when called in onDestroy

hi , i am facing another issue , when i am trying to pause all downloads before closing the app , " downloadManager.pauseBatch(download.getBatchId());" not working in onDestroy methode , i mean it doesn't pause the downloads
please help
thx in advance

Improve set-up for library

Currently requires a class which is accessed through reflection and a manifest placeholder. Consider using a Gradle plug-in or similar to do this?

app crashes when press on notification after complete

hi , a new issue appeared to me , when i press on notification after download finish the app crash
hope to fix this soon
thx in advance

Improve demo apps

• Split each feature in the extended demo into its own activity (makes it more obvious)
• Add a switch to show downloads or batches in the list
• Prettify
• multiple adapters per package so each activity is understandable on it's own
• Add READMEs inside specific packages explaining demos

Anything else?

What unfortunate Authority class is!

I totally don't understand why there need a com.novoda.downloadmanager.Authority class!

You also need to create this file in this exact package (we hope to do this for you in the future):

I think it time to do this for us! Look forward!Thanks very much.:)

If two apps are installed with this lib they'll clash contentprovider names

Meaning the second won't work?

https://github.com/novoda/download-manager/blob/master/library/src/main/AndroidManifest.xml#L14-L17

Discarding a the notification of a cancelled download adds a new notification

Steps to reproduce:

1. start simple demo and download a file
2. use notification to cancel download
3. try to discard the notification for the cancelled download

See that the same notification is re-added.

Remove system DOWNLOAD_WITHOUT_NOTIFICATION permission

Currently downloading without a notification uses a system permission - but this download manager is not a system service (unlike the AOSP one). We should investigate if it is required, or at the very least namespace the permission to a novoda namespace

Clicking on notification after download complete crashes the demo-simple app

1. Open the demo-simple app.
2. Download the podcast
3. Once the download is finished, click on the "Family of Penguins - Download Complete" notification.

I originally saw this in my app and wanted to check if the demo apps behaved any differently.

07-27 09:51:17.268  19439-19614/com.novoda.downloadmanager.demo.simple E/AndroidRuntime﹕ FATAL EXCEPTION: DownloadReceiver
    Process: com.novoda.downloadmanager.demo.simple, PID: 19439
    java.lang.SecurityException: Provider com.novoda.downloadmanager.demo.simple/com.novoda.downloadmanager.lib.DownloadProvider does not allow granting of Uri permissions (uri content://com.novoda.downloadmanager.demo.simple/all_downloads/1)
            at android.os.Parcel.readException(Parcel.java:1465)
            at android.os.Parcel.readException(Parcel.java:1419)
            at android.app.ActivityManagerProxy.startActivity(ActivityManagerNative.java:2096)
            at android.app.Instrumentation.execStartActivity(Instrumentation.java:1419)
            at android.app.ContextImpl.startActivity(ContextImpl.java:1047)
            at android.app.ContextImpl.startActivity(ContextImpl.java:1029)
            at android.content.ContextWrapper.startActivity(ContextWrapper.java:311)
            at android.content.ContextWrapper.startActivity(ContextWrapper.java:311)
            at com.novoda.downloadmanager.lib.DownloadReceiver.openDownload(DownloadReceiver.java:159)
            at com.novoda.downloadmanager.lib.DownloadReceiver.handleNotificationBroadcast(DownloadReceiver.java:108)
            at com.novoda.downloadmanager.lib.DownloadReceiver.access$000(DownloadReceiver.java:31)
            at com.novoda.downloadmanager.lib.DownloadReceiver$1.run(DownloadReceiver.java:92)
            at android.os.Handler.handleCallback(Handler.java:733)
            at android.os.Handler.dispatchMessage(Handler.java:95)
            at android.os.Looper.loop(Looper.java:136)
            at android.os.HandlerThread.run(HandlerThread.java:61)

Implement a default NotificationImageRetriever for lazy ppl

https://github.com/novoda/download-manager/blob/master/library/src/main/java/com/novoda/downloadmanager/lib/NotificationImageRetriever.java

DownloadServices crashes after last download deleted

After the last download has been deleted the DownloadService sometimes crashes with

07-14 15:33:09.832   6476-24557/com.novoda.downloadmanager.demo.extended W/NoTils﹕ [pool-29-thread-1][Log.w:99] Exception for id 62: Index 0 requested, with a size of 0
    android.database.CursorIndexOutOfBoundsException: Index 0 requested, with a size of 0
            at android.database.AbstractCursor.checkPosition(AbstractCursor.java:426)
            at android.database.AbstractWindowedCursor.checkPosition(AbstractWindowedCursor.java:136)
            at android.database.AbstractWindowedCursor.getLong(AbstractWindowedCursor.java:74)
            at android.database.CursorWrapper.getLong(CursorWrapper.java:106)
            at com.novoda.downloadmanager.lib.FileDownloadInfo$Reader.getLong(FileDownloadInfo.java:449)
            at com.novoda.downloadmanager.lib.FileDownloadInfo$Reader.updateFromDatabase(FileDownloadInfo.java:378)
            at com.novoda.downloadmanager.lib.FileDownloadInfo$Reader.newDownloadInfo(FileDownloadInfo.java:371)
            at com.novoda.downloadmanager.lib.DownloadService.createNewDownloadInfo(DownloadService.java:181)
            at com.novoda.downloadmanager.lib.DownloadService.access$100(DownloadService.java:59)
            at com.novoda.downloadmanager.lib.DownloadService$1.create(DownloadService.java:170)
            at com.novoda.downloadmanager.lib.DownloadsRepository.getDownloadFor(DownloadsRepository.java:45)
            at com.novoda.downloadmanager.lib.DownloadThread.checkPausedOrCanceled(DownloadThread.java:574)
            at com.novoda.downloadmanager.lib.DownloadThread.transferData(DownloadThread.java:538)
            at com.novoda.downloadmanager.lib.DownloadThread.transferData(DownloadThread.java:456)
            at com.novoda.downloadmanager.lib.DownloadThread.executeDownload(DownloadThread.java:370)
            at com.novoda.downloadmanager.lib.DownloadThread.runInternal(DownloadThread.java:274)
            at com.novoda.downloadmanager.lib.DownloadThread.run(DownloadThread.java:197)

Status of batch should not go to Queued between items

Currently the status of a batch will go from downloading, queued, and then downloading again when it finishes one download but is about to start another. It'd be good if this didn't update the batch status as "queued" in the middle, giving the user a more consistent experience

Remove the URI from the Intent used for the click events on Notifications

https://github.com/novoda/download-manager/pull/167/files#diff-14235d3c65570f071b2574801705fc2aR128

That Uri seems like it isn't used anywhere so we should make sure it is not and then get rid of it.

Notification is shown after batch quickly deleted

In demo-extend:
A download notification is shown after the user quickly the delete button after adding a new download, i.e. before the download notification is shown.

This can be reproduced by setting a break point e.g. in DownloadNotifier.buildNotificationTag

Failure to unlock stale downloads on Service startup

When the service starts it tries to unlock stale downloads by fetching the current batches list using getCurrentDownloadingOrSubmittedBatchIds

There is a bad sqlite statement in this method that never returns anything. =>

download-manager/library/src/main/java/com/novoda/downloadmanager/lib/DownloadsRepository.java

Line 207 in cad0503

+ "GROUP BY (" + DownloadContract.Downloads.COLUMN_BATCH_ID;

Define a Download

Currently we have

• FileDownloadInfo The uri which is enqueued, given an id and downloaded
• DownloadBatch The id that groups FileDownloadInfo ids together
• Download The publicly exposed model which exposes the DownloadBatch information (such as total size)

We need better define the difference in these and whether or not the public API should know about a "batch"

AvoidFieldNameMatchingTypeName

Authority.java:6, AvoidFieldNameMatchingTypeName, Priority: Normal
It is somewhat confusing to have a field name matching the declaring class name.

It is somewhat confusing to have a field name matching the declaring class name. This probably means that type and/or field names should be chosen more carefully.

public class Foo extends Bar {
    int foo;    // There is probably a better name that can be used
}

Downloads show as pending between file downloads

When we query the status of a batch after a file has finished downloading and before the next one starts downloading, the batch will have a queued status.

Remove dependency between supplying NotificationImageRetriever & Application class

https://github.com/novoda/download-manager/blob/master/library/src/main/java/com/novoda/downloadmanager/lib/DownloadService.java#L161

:library:processDebugAndroidTestManifest fails in Android Studio

When user has selected test artifact Android instrumentation test the sync fails with

Error:(21, 13) Attribute provider#com.novoda.downloadmanager.lib.DownloadProvider@authorities at debug:21:13 requires a placeholder substitution but no value for <downloadAuthority> is provided.

Users might not aware of their settings. The library should provide a value for AndroidTest