Skip to content

Commit 9c8b674

Browse files
annzimmerannzimmer
committed
more user logs.
1 parent 9ae9694 commit 9c8b674

File tree

5 files changed

+51
-21
lines changed

5 files changed

+51
-21
lines changed

‎firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/FirebaseMlException.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ public class FirebaseMlException extends FirebaseException {
101101
/** There is not enough space left on the device. */
102102
public static final int NOT_ENOUGH_SPACE = 101;
103103

104+
/** The downloaded model's hash doesn't match the expected value. */
105+
public static final int MODEL_HASH_MISMATCH = 102;
106+
104107
/**
105108
* These download url expired before download could complete. Usually, multiple download attempt
106109
* will be performed before this is returned.
@@ -129,6 +132,7 @@ public class FirebaseMlException extends FirebaseException {
129132
UNAVAILABLE,
130133
UNAUTHENTICATED,
131134
NOT_ENOUGH_SPACE,
135+
MODEL_HASH_MISMATCH,
132136
DOWNLOAD_URL_EXPIRED
133137
})
134138
@Retention(RetentionPolicy.CLASS)

‎firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/FirebaseModelDownloader.java

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package com.google.firebase.ml.modeldownloader;
1515

1616
import android.os.Build.VERSION_CODES;
17+
import android.util.Log;
1718
import androidx.annotation.NonNull;
1819
import androidx.annotation.Nullable;
1920
import androidx.annotation.RequiresApi;
@@ -39,6 +40,7 @@
3940

4041
public class FirebaseModelDownloader {
4142

43+
private static final String TAG = "FirebaseModelDownld";
4244
private final FirebaseOptions firebaseOptions;
4345
private final SharedPreferencesUtil sharedPreferencesUtil;
4446
private final ModelFileDownloadService fileDownloadService;
@@ -247,7 +249,7 @@ private Task<CustomModel> getCustomModelTask(
247249
CustomModel currentModel = sharedPreferencesUtil.getCustomModelDetails(modelName);
248250

249251
if (currentModel == null && modelHash != null) {
250-
// todo(annzimmer) log something about mismatched state and use hash = null
252+
Log.d(TAG, "Model hash provided but no current model; triggering fresh download.");
251253
modelHash = null;
252254
}
253255

@@ -305,16 +307,34 @@ && new File(currentModel.getLocalFilePath()).exists()) {
305307
if (currentModel.getDownloadId() != 0) {
306308
CustomModel downloadingModel =
307309
sharedPreferencesUtil.getDownloadingCustomModelDetails(modelName);
308-
if (downloadingModel != null
309-
&& downloadingModel
310-
.getModelHash()
311-
.equals(incomingModelDetails.getResult().getModelHash())) {
312-
return Tasks.forResult(downloadingModel);
310+
if (downloadingModel != null) {
311+
if (downloadingModel
312+
.getModelHash()
313+
.equals(incomingModelDetails.getResult().getModelHash())) {
314+
return Tasks.forResult(downloadingModel);
315+
}
316+
Log.d(
317+
TAG, "Hash does not match with expected: " + downloadingModel.getModelHash());
318+
// Note we log "DownloadStatus.SUCCEEDED" because the model file's download itself
319+
// succeeded. Just the hash validation failed.
320+
eventLogger.logDownloadEventWithErrorCode(
321+
downloadingModel,
322+
true,
323+
DownloadStatus.SUCCEEDED,
324+
ErrorCode.MODEL_HASH_MISMATCH);
325+
return Tasks.forException(
326+
new FirebaseMlException(
327+
"Hash does not match with expected",
328+
FirebaseMlException.MODEL_HASH_MISMATCH));
313329
}
314-
// todo(annzimmer) this shouldn't happen unless they are calling the sdk with
315-
// multiple
316-
// sets of download types/conditions.
317-
// this should be a download in progress - add appropriate handling.
330+
Log.d(TAG, "Download details missing for model");
331+
// Note we log "DownloadStatus.SUCCEEDED" because the model file's download itself
332+
// succeeded. Just the file copy failed.
333+
eventLogger.logDownloadEventWithErrorCode(
334+
downloadingModel, true, DownloadStatus.SUCCEEDED, ErrorCode.DOWNLOAD_FAILED);
335+
return Tasks.forException(
336+
new FirebaseMlException(
337+
"Download details missing for model", FirebaseMlException.INTERNAL));
318338
}
319339
}
320340

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@ public static DataTransportMlEventSender create(TransportFactory transportFactor
4848
}
4949

5050
public void sendEvent(@NonNull FirebaseMlLogEvent firebaseMlLogEvent) {
51-
// TODO Use .send or .schedule - which gives back task of logging progress?
52-
// Not sure how strongly we feel about tracking these? I'm happy to use plain send.
5351
transport.send(Event.ofData(firebaseMlLogEvent));
5452
}
5553
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ public enum ErrorCode {
148148
DOWNLOAD_FAILED(104),
149149
MODEL_INFO_DOWNLOAD_UNSUCCESSFUL_HTTP_STATUS(105),
150150
MODEL_INFO_DOWNLOAD_CONNECTION_FAILED(107),
151+
MODEL_HASH_MISMATCH(116),
151152
UNKNOWN_ERROR(9999);
152153
private static final SparseArray<ErrorCode> valueMap = new SparseArray<>();
153154

@@ -160,6 +161,7 @@ public enum ErrorCode {
160161
valueMap.put(104, DOWNLOAD_FAILED);
161162
valueMap.put(105, MODEL_INFO_DOWNLOAD_UNSUCCESSFUL_HTTP_STATUS);
162163
valueMap.put(107, MODEL_INFO_DOWNLOAD_CONNECTION_FAILED);
164+
valueMap.put(116, MODEL_HASH_MISMATCH);
163165
valueMap.put(9999, UNKNOWN_ERROR);
164166
}
165167

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,6 @@ public static ModelFileDownloadService getInstance() {
116116
public Task<Void> download(
117117
CustomModel customModel, CustomModelDownloadConditions downloadConditions) {
118118
this.downloadConditions = downloadConditions;
119-
// todo add url tests here
120119
return ensureModelDownloaded(customModel);
121120
}
122121

@@ -142,6 +141,7 @@ && existTaskCompletionSourceInstance(downloadingModel.getDownloadId())) {
142141
> (now.getTime() - COMPLETION_BUFFER_IN_MS)))) {
143142
// download in progress - return this task result.
144143

144+
Log.d(TAG, "New model is already in downloading, return existing task.");
145145
eventLogger.logDownloadEventWithErrorCode(
146146
downloadingModel, false, DownloadStatus.DOWNLOADING, ErrorCode.NO_ERROR);
147147
return getExistingDownloadTask(downloadingModel.getDownloadId());
@@ -153,6 +153,7 @@ && existTaskCompletionSourceInstance(downloadingModel.getDownloadId())) {
153153
}
154154

155155
// schedule new download of model file
156+
Log.d(TAG, "Need to download a new model.");
156157
Long newDownloadId = scheduleModelDownload(customModel);
157158
if (newDownloadId == null) {
158159
return Tasks.forException(
@@ -236,6 +237,7 @@ synchronized boolean existTaskCompletionSourceInstance(long downloadId) {
236237
@VisibleForTesting
237238
synchronized Long scheduleModelDownload(@NonNull CustomModel customModel) {
238239
if (downloadManager == null) {
240+
Log.d(TAG, "Download manager service is not available in the service.");
239241
return null;
240242
}
241243

@@ -262,6 +264,7 @@ synchronized Long scheduleModelDownload(@NonNull CustomModel customModel) {
262264
}
263265

264266
long id = downloadManager.enqueue(downloadRequest);
267+
Log.d(TAG, "Schedule a new downloading task: " + id);
265268
// update the custom model to store the download id - do not lose current local file - in case
266269
// this is a background update.
267270
CustomModel model =
@@ -318,8 +321,7 @@ private synchronized ParcelFileDescriptor getDownloadedFile(Long downloadingId)
318321
try {
319322
fileDescriptor = downloadManager.openDownloadedFile(downloadingId);
320323
} catch (FileNotFoundException e) {
321-
// todo replace with FirebaseMlException
322-
Log.d(TAG, "Downloaded file is not found: " + e);
324+
Log.d(TAG, "Downloaded file is not found.");
323325
}
324326
return fileDescriptor;
325327
}
@@ -379,14 +381,13 @@ public File loadNewlyDownloadedModelFile(CustomModel model) {
379381
if (fileDescriptor == null) {
380382
// reset original model - removing download id.
381383
removeOrCancelDownloadModel(model.getName(), model.getDownloadId());
382-
// todo log this?
383384
return null;
384385
}
385386

386387
// Try to move it to destination folder.
387388
File newModelFile;
388389
try {
389-
// TODO add logging
390+
Log.d(TAG, "Moving downloaded model from external storage to destination folder.");
390391
newModelFile = fileManager.moveModelToDestinationFolder(model, fileDescriptor);
391392
} catch (FirebaseMlException ex) {
392393
// add logging for this error
@@ -399,6 +400,10 @@ public File loadNewlyDownloadedModelFile(CustomModel model) {
399400
return null;
400401
}
401402

403+
Log.d(
404+
TAG,
405+
"Moved the downloaded model to destination folder successfully: "
406+
+ newModelFile.getParent());
402407
// Successfully moved, update share preferences
403408
sharedPreferencesUtil.setLoadedCustomModelDetails(
404409
new CustomModel(
@@ -508,6 +513,10 @@ public void onReceive(Context context, Intent intent) {
508513
// Our current code does not have this problem. However, in order to be safer in the
509514
// future, we just ignore the exception here, because it is not a big deal. The code can
510515
// move on.
516+
Log.w(
517+
TAG,
518+
"Exception thrown while trying to unregister the broadcast receiver for the download",
519+
e);
511520
}
512521

513522
removeDownloadTaskInstance(downloadId);
@@ -522,9 +531,7 @@ public void onReceive(Context context, Intent intent) {
522531
if (downloadingModel != null) {
523532
eventLogger.logDownloadFailureWithReason(downloadingModel, false, failureReason);
524533
if (checkErrorCausedByExpiry(downloadingModel.getDownloadUrlExpiry(), failureReason)) {
525-
// retry as a new download
526-
// todo change to FirebaseMlException retry error - or whatever we decide is
527-
// appropriate.
534+
// this error will trigger a specific number of retries.
528535
taskCompletionSource.setException(
529536
new FirebaseMlException(
530537
"Retry: Expired URL for id: " + downloadingModel.getDownloadId(),
@@ -541,7 +548,6 @@ public void onReceive(Context context, Intent intent) {
541548
// model update might have been completed already get the downloaded model.
542549
downloadingModel = sharedPreferencesUtil.getCustomModelDetails(modelName);
543550
if (downloadingModel == null) {
544-
// todo add logging here.
545551
taskCompletionSource.setException(
546552
new FirebaseMlException(
547553
"No model associated with name: " + modelName,

0 commit comments

Comments
 (0)