Add Ml shared preferences for custom model storage #2104
Conversation
Coverage ReportAffected SDKs
Test Logs
NotesHTML coverage reports can be produced locally with Head commit (b0e24228) is created by Prow via merging commits: 844ff28 33eed38. |
Binary Size ReportAffected SDKs
Test Logs
NotesHead commit (b0e24228) is created by Prow via merging commits: 844ff28 33eed38. |
|
/retest |
1 similar comment
|
/retest |
| */ | ||
| public CustomModel( | ||
| @NonNull String name, long downloadId, long fileSize, @NonNull String modelHash) { | ||
| this.modelHash = modelHash; |
There was a problem hiding this comment.
You could call the other constructor from here and assignments centralized in a single method.
| @@ -76,18 +118,31 @@ public long getDownloadId() { | |||
| return downloadId; | |||
| } | |||
|
|
|||
| @Override | |||
| public boolean equals(Object o) { | |||
There was a problem hiding this comment.
Override hashCode when overriding equals (Item 11 in Effective Java)
| * model is present - this returns the details of that model, including local file path. If an | ||
| * update of an existing model is in progress, the local model plus the download id for the new | ||
| * upload is returned. To get only details related to the downloading model use | ||
| * getDownloadingCustomModelDetails. If this is the initial download of a local file - the |
There was a problem hiding this comment.
| * getDownloadingCustomModelDetails. If this is the initial download of a local file - the | |
| * {@link #getDownloadingCustomModelDetails}. If this is the initial download of a local file - the |
| private static final CustomModel CUSTOM_MODEL_DOWNLOADING = | ||
| new CustomModel(MODEL_NAME, 986, 100, MODEL_HASH); | ||
|
|
||
| // private static final FirebaseApp FIREBASE_APP = FirebaseApp.initializeApp( |
| String.format(LOCAL_MODEL_FILE_PATH_PATTERN, persistenceKey, modelName), filePath) | ||
| .apply(); | ||
|
|
||
| clearDownloadingModelDetails(customModel.getName()); |
There was a problem hiding this comment.
Would it be useful to do the clearing as part of the change above? If yes, then you can pass the editor to clearDownloadingModelDetails and use it to add the remove statements in a single apply() call.
| .putLong( | ||
| String.format(DOWNLOAD_BEGIN_TIME_MS_PATTERN, persistenceKey, modelName), | ||
| SystemClock.elapsedRealtime()) | ||
| .apply(); |
There was a problem hiding this comment.
apply() only alters the in-memory contents of the SharedPreferences, and eventually writes to disk, which may fail silently, unlike commit() that's synchronous. Just want to double check that this is an acceptable situation in this case.
There was a problem hiding this comment.
changed to commit
No description provided.