Skip to content

Commit 389782d

Browse files
committed
Clean up/remove failed or completed download attempts.
Removes old download attempts from the download managers and resets the models in shared preferences.
1 parent cdf06e2 commit 389782d

File tree

2 files changed

+294
-43
lines changed

2 files changed

+294
-43
lines changed

firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/internal/ModelFileDownloadService.java

Lines changed: 90 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import android.os.Build.VERSION;
2727
import android.os.Build.VERSION_CODES;
2828
import android.os.ParcelFileDescriptor;
29+
import android.util.Log;
2930
import android.util.LongSparseArray;
3031
import androidx.annotation.GuardedBy;
3132
import androidx.annotation.NonNull;
@@ -57,6 +58,9 @@
5758
*/
5859
public class ModelFileDownloadService {
5960

61+
private static final String TAG = "ModelFileDownloadSer";
62+
private static final int COMPLETION_BUFFER_IN_MS = 60 * 5 * 1000;
63+
6064
private final DownloadManager downloadManager;
6165
private final Context context;
6266
private final ModelFileManager fileManager;
@@ -125,10 +129,30 @@ public Task<Void> download(
125129

126130
@VisibleForTesting
127131
Task<Void> ensureModelDownloaded(CustomModel customModel) {
128-
// todo check model not already in progress of being downloaded
129-
130-
// todo remove any failed download attempts
131-
132+
// todo add logging for explicitly requested
133+
// check model download already in progress
134+
CustomModel downloadingModel =
135+
sharedPreferencesUtil.getDownloadingCustomModelDetails(customModel.getName());
136+
if (downloadingModel != null && downloadingModel.getDownloadId() != 0) {
137+
Integer statusCode = getDownloadingModelStatusCode(downloadingModel.getDownloadId());
138+
Date now = new Date();
139+
140+
// check if download has completed or still has time to finish.
141+
// Give a buffer above url expiry to continue if in progress.
142+
if (statusCode != null
143+
&& (statusCode == DownloadManager.STATUS_SUCCESSFUL
144+
|| statusCode == DownloadManager.STATUS_FAILED
145+
|| (customModel.getDownloadUrlExpiry()
146+
> (now.getTime() - COMPLETION_BUFFER_IN_MS)))) {
147+
// download in progress for this hash already - return this result.
148+
return registerReceiverForDownloadId(
149+
downloadingModel.getDownloadId(), customModel.getName());
150+
}
151+
}
152+
if (downloadingModel != null) {
153+
// remove failed download attempts
154+
removeOrCancelDownload(downloadingModel.getName(), downloadingModel.getDownloadId());
155+
}
132156
// schedule new download of model file
133157
Long newDownloadId = scheduleModelDownload(customModel);
134158
if (newDownloadId == null) {
@@ -138,6 +162,14 @@ Task<Void> ensureModelDownloaded(CustomModel customModel) {
138162
return registerReceiverForDownloadId(newDownloadId, customModel.getName());
139163
}
140164

165+
/** Removes or cancels the downloading model if exists. */
166+
synchronized void removeOrCancelDownload(String modelName, Long downloadId) {
167+
if (downloadManager != null && downloadId != 0) {
168+
downloadManager.remove(downloadId);
169+
}
170+
sharedPreferencesUtil.setFailedUploadedCustomModelDetails(modelName);
171+
}
172+
141173
private synchronized DownloadBroadcastReceiver getReceiverInstance(
142174
long downloadId, String modelName) {
143175
DownloadBroadcastReceiver receiver = receiverMaps.get(downloadId);
@@ -253,8 +285,8 @@ private synchronized ParcelFileDescriptor getDownloadedFile(Long downloadingId)
253285
try {
254286
fileDescriptor = downloadManager.openDownloadedFile(downloadingId);
255287
} catch (FileNotFoundException e) {
256-
// todo(annz)
257-
System.out.println("Downloaded file is not found");
288+
// todo replace with FirebaseMlException
289+
Log.d(TAG, "Downloaded file is not found" + e);
258290
}
259291
return fileDescriptor;
260292
}
@@ -286,12 +318,17 @@ public File loadNewlyDownloadedModelFile(CustomModel model) throws FirebaseMlExc
286318
String downloadingModelHash = model.getModelHash();
287319

288320
if (downloadingId == 0 || downloadingModelHash.isEmpty()) {
289-
// no downloading model file or incomplete info.
321+
// Clear the downloading info completely.
322+
// It handles the case: developer clear the app cache but downloaded model file in
323+
// DownloadManager's cache would not be cleared.
324+
removeOrCancelDownload(model.getName(), model.getDownloadId());
290325
return null;
291326
}
292327

293328
Integer statusCode = getDownloadingModelStatusCode(downloadingId);
294329
if (statusCode == null) {
330+
// No status code, it may mean no such download or no download manager.
331+
removeOrCancelDownload(model.getName(), model.getDownloadId());
295332
return null;
296333
}
297334

@@ -301,16 +338,20 @@ public File loadNewlyDownloadedModelFile(CustomModel model) throws FirebaseMlExc
301338
if (fileDescriptor == null) {
302339
// reset original model - removing download id.
303340
sharedPreferencesUtil.setFailedUploadedCustomModelDetails(model.getName());
304-
// todo call the download register?
341+
removeOrCancelDownload(model.getName(), model.getDownloadId());
342+
// todo log this?
305343
return null;
306344
}
307345

308346
// Try to move it to destination folder.
309-
File newModelFile = fileManager.moveModelToDestinationFolder(model, fileDescriptor);
347+
File newModelFile;
348+
try {
349+
newModelFile = fileManager.moveModelToDestinationFolder(model, fileDescriptor);
350+
} finally {
351+
removeOrCancelDownload(model.getName(), model.getDownloadId());
352+
}
310353

311354
if (newModelFile == null) {
312-
// reset original model - removing download id.
313-
// todo call the download register?
314355
sharedPreferencesUtil.setFailedUploadedCustomModelDetails(model.getName());
315356
return null;
316357
}
@@ -323,9 +364,9 @@ public File loadNewlyDownloadedModelFile(CustomModel model) throws FirebaseMlExc
323364
// todo(annzimmer) Cleans up the old files if it is the initial creation.
324365
return newModelFile;
325366
} else if (statusCode == DownloadManager.STATUS_FAILED) {
326-
// reset original model - removing download id.
367+
// reset original model - removing downloading details.
327368
sharedPreferencesUtil.setFailedUploadedCustomModelDetails(model.getName());
328-
// todo - determine if the temp files need to be clean up? Does one exist?
369+
removeOrCancelDownload(model.getName(), model.getDownloadId());
329370
}
330371
// Other cases, return as null and wait for download finish.
331372
return null;
@@ -398,6 +439,13 @@ public void onReceive(Context context, Intent intent) {
398439
}
399440

400441
Integer statusCode = getDownloadingModelStatusCode(downloadId);
442+
// check to prevent DuplicateTaskCompletionException - this was already updated and removed.
443+
// Just return.
444+
if (taskCompletionSourceMaps.get(downloadId) == null) {
445+
receiverMaps.remove(downloadId);
446+
return;
447+
}
448+
401449
synchronized (ModelFileDownloadService.this) {
402450
try {
403451
context.getApplicationContext().unregisterReceiver(this);
@@ -415,25 +463,39 @@ public void onReceive(Context context, Intent intent) {
415463

416464
if (statusCode != null) {
417465
if (statusCode == DownloadManager.STATUS_FAILED) {
418-
eventLogger.logDownloadFailureWithReason(
419-
sharedPreferencesUtil.getDownloadingCustomModelDetails(modelName),
420-
false,
421-
getFailureReason(id));
422-
if (checkErrorCausedByExpiry(id, modelName)) {
423-
// retry as a new download
424-
// todo change to FirebaseMlException retry error.
425-
taskCompletionSource.setException(new Exception("Retry: Expired URL"));
426-
return;
466+
int failureReason = getFailureReason(id);
467+
CustomModel downloadingModel =
468+
sharedPreferencesUtil.getDownloadingCustomModelDetails(modelName);
469+
if (downloadingModel != null) {
470+
eventLogger.logDownloadFailureWithReason(downloadingModel, false, failureReason);
471+
if (checkErrorCausedByExpiry(downloadingModel.getDownloadUrlExpiry(), failureReason)) {
472+
// retry as a new download
473+
// todo change to FirebaseMlException retry error - or whatever we decide is
474+
// appropriate.
475+
taskCompletionSource.setException(new Exception("Retry: Expired URL"));
476+
return;
477+
}
427478
}
428479
taskCompletionSource.setException(getExceptionAccordingToDownloadManager(id));
429480
return;
430481
}
431482

432483
if (statusCode == DownloadManager.STATUS_SUCCESSFUL) {
484+
CustomModel model = sharedPreferencesUtil.getDownloadingCustomModelDetails(modelName);
485+
if (model == null) {
486+
// model might have been updated already get the downloaded model.
487+
model = sharedPreferencesUtil.getCustomModelDetails(modelName);
488+
if (model == null) {
489+
// todo add logging here.
490+
taskCompletionSource.setException(
491+
new FirebaseMlException(
492+
"No model associated with name: " + modelName,
493+
FirebaseMlException.INVALID_ARGUMENT));
494+
return;
495+
}
496+
}
433497
eventLogger.logDownloadEventWithExactDownloadTime(
434-
sharedPreferencesUtil.getDownloadingCustomModelDetails(modelName),
435-
ErrorCode.NO_ERROR,
436-
DownloadStatus.SUCCEEDED);
498+
model, ErrorCode.NO_ERROR, DownloadStatus.SUCCEEDED);
437499
taskCompletionSource.setResult(null);
438500
return;
439501
}
@@ -443,25 +505,11 @@ public void onReceive(Context context, Intent intent) {
443505
taskCompletionSource.setException(new Exception("Model downloading failed"));
444506
}
445507

446-
private boolean checkErrorCausedByExpiry(Long downloadId, String modelName) {
447-
CustomModel model = sharedPreferencesUtil.getCustomModelDetails(modelName);
448-
449-
if (model == null) {
450-
return false;
451-
}
452-
508+
private boolean checkErrorCausedByExpiry(long downloadUrlExpiry, int failureReason) {
453509
final Date time = new Date();
454510

455-
if (model.getDownloadUrlExpiry() < time.getTime()) {
456-
Cursor cursor =
457-
(downloadManager == null || downloadId == null)
458-
? null
459-
: downloadManager.query(new Query().setFilterById(downloadId));
460-
if (cursor != null && cursor.moveToFirst()) {
461-
int reason = cursor.getInt(cursor.getColumnIndex(DownloadManager.COLUMN_REASON));
462-
// 400 implies possibility of url expiry
463-
return (reason == 400);
464-
}
511+
if (failureReason == 400 && downloadUrlExpiry < time.getTime()) {
512+
return true;
465513
}
466514
return false;
467515
}

0 commit comments

Comments
 (0)