Skip to content

Commit 5220062

Browse files
annzimmerannzimmer
committed
Updates FirebaseMlLogEvent ModelOption name to Options to make internal proto. This allows for proper json to proto conversion and in local tests messages now reach the spanner queue.
1 parent 90018d9 commit 5220062

File tree

2 files changed

+61
-15
lines changed

2 files changed

+61
-15
lines changed

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import com.google.firebase.ml.modeldownloader.internal.ModelFileDownloadService;
3030
import com.google.firebase.ml.modeldownloader.internal.ModelFileManager;
3131
import com.google.firebase.ml.modeldownloader.internal.SharedPreferencesUtil;
32+
import java.io.File;
3233
import java.util.Set;
3334
import java.util.concurrent.Executor;
3435
import java.util.concurrent.Executors;
@@ -130,11 +131,10 @@ public Task<CustomModel> getModel(
130131
return Tasks.forResult(localModel);
131132
case LATEST_MODEL:
132133
// check for latest model, wait for download if newer model exists
133-
return getCustomModelTask(modelName, conditions);
134+
return getCustomModelTask(modelName, conditions, localModel.getModelHash());
134135
case LOCAL_MODEL_UPDATE_IN_BACKGROUND:
135-
// start download in back ground, return current model
136+
// start download in back ground, return local model
136137
getCustomModelTask(modelName, conditions, localModel.getModelHash());
137-
// return local model
138138
return Tasks.forResult(localModel);
139139
}
140140
throw new IllegalArgumentException(
@@ -166,15 +166,18 @@ private Task<CustomModel> getCustomModelTask(
166166
if (incomingModelDetailTask.isSuccessful()) {
167167
CustomModel currentModel = sharedPreferencesUtil.getCustomModelDetails(modelName);
168168
// null means we have the latest model
169-
if (incomingModelDetails.getResult() == null) {
169+
if (incomingModelDetailTask.getResult() == null) {
170170
return Tasks.forResult(currentModel);
171171
}
172172

173173
// if modelHash matches current local model just return local model.
174-
if (currentModel
175-
.getModelHash()
176-
.equals(incomingModelDetails.getResult().getModelHash())) {
177-
if (!currentModel.getLocalFilePath().isEmpty()) {
174+
// Should be handled by above case but just in case.
175+
if (currentModel != null
176+
&& currentModel
177+
.getModelHash()
178+
.equals(incomingModelDetails.getResult().getModelHash())) {
179+
if (!currentModel.getLocalFilePath().isEmpty()
180+
&& new File(currentModel.getLocalFilePath()).exists()) {
178181
return Tasks.forResult(currentModel);
179182
}
180183
// todo(annzimmer) this shouldn't happen unless they are calling the sdk with multiple

‎firebase-ml-modeldownloader/src/test/java/com/google/firebase/ml/modeldownloader/FirebaseModelDownloaderTest.java

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ public void teardown() {
171171
public void getModel_latestModel_localExists_noUpdate() throws Exception {
172172
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME))).thenReturn(customModelLoaded);
173173
when(mockModelDownloadService.getCustomModelDetails(
174-
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(null)))
174+
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(MODEL_HASH)))
175175
.thenReturn(Tasks.forResult(CUSTOM_MODEL)); // no change found
176176

177177
TestOnCompleteListener<CustomModel> onCompleteListener = new TestOnCompleteListener<>();
@@ -186,11 +186,41 @@ public void getModel_latestModel_localExists_noUpdate() throws Exception {
186186
assertEquals(customModel, customModelLoaded);
187187
}
188188

189+
@Test
190+
public void getModel_latestModel_localExists_noUpdate_MissingFile() throws Exception {
191+
// model with missing file.
192+
CustomModel missingFileModel =
193+
new CustomModel(MODEL_NAME, UPDATE_MODEL_HASH, 100, 0, expectedDestinationFolder + "/4");
194+
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME)))
195+
.thenReturn(missingFileModel)
196+
.thenReturn(missingFileModel)
197+
.thenReturn(customModelUploaded);
198+
199+
when(mockModelDownloadService.getCustomModelDetails(
200+
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(UPDATE_MODEL_HASH)))
201+
.thenReturn(Tasks.forResult(UPDATE_CUSTOM_MODEL_URL));
202+
when(mockFileDownloadService.download(any(), eq(DEFAULT_DOWNLOAD_CONDITIONS)))
203+
.thenReturn(Tasks.forResult(null));
204+
when(mockFileDownloadService.loadNewlyDownloadedModelFile(eq(customModelUploaded)))
205+
.thenReturn(secondDeviceModelFile);
206+
207+
TestOnCompleteListener<CustomModel> onCompleteListener = new TestOnCompleteListener<>();
208+
Task<CustomModel> task =
209+
firebaseModelDownloader.getModel(
210+
MODEL_NAME, DownloadType.LATEST_MODEL, DEFAULT_DOWNLOAD_CONDITIONS);
211+
task.addOnCompleteListener(executor, onCompleteListener);
212+
CustomModel customModel = onCompleteListener.await();
213+
214+
verify(mockPrefs, times(3)).getCustomModelDetails(eq(MODEL_NAME));
215+
assertThat(task.isComplete()).isTrue();
216+
assertEquals(customModel, customModelUploaded);
217+
}
218+
189219
@Test
190220
public void getModel_latestModel_localExists_sameHash() throws Exception {
191221
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME))).thenReturn(customModelLoaded);
192222
when(mockModelDownloadService.getCustomModelDetails(
193-
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(null)))
223+
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(MODEL_HASH)))
194224
.thenReturn(Tasks.forResult(CUSTOM_MODEL)); // no change found
195225

196226
TestOnCompleteListener<CustomModel> onCompleteListener = new TestOnCompleteListener<>();
@@ -211,8 +241,9 @@ public void getModel_latestModel_localExists_UpdateFound() throws Exception {
211241
.thenReturn(customModelLoaded)
212242
.thenReturn(customModelLoaded)
213243
.thenReturn(customModelUploaded);
244+
214245
when(mockModelDownloadService.getCustomModelDetails(
215-
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(null)))
246+
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(MODEL_HASH)))
216247
.thenReturn(Tasks.forResult(UPDATE_CUSTOM_MODEL_URL));
217248
when(mockFileDownloadService.download(any(), eq(DEFAULT_DOWNLOAD_CONDITIONS)))
218249
.thenReturn(Tasks.forResult(null));
@@ -233,7 +264,10 @@ public void getModel_latestModel_localExists_UpdateFound() throws Exception {
233264

234265
@Test
235266
public void getModel_latestModel_noLocalModel() throws Exception {
236-
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME))).thenReturn(null).thenReturn(CUSTOM_MODEL);
267+
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME)))
268+
.thenReturn(null)
269+
.thenReturn(null)
270+
.thenReturn(CUSTOM_MODEL);
237271
when(mockModelDownloadService.getCustomModelDetails(
238272
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(null)))
239273
.thenReturn(Tasks.forResult(CUSTOM_MODEL));
@@ -255,7 +289,10 @@ public void getModel_latestModel_noLocalModel() throws Exception {
255289

256290
@Test
257291
public void getModel_latestModel_noLocalModel_error() throws Exception {
258-
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME))).thenReturn(null).thenReturn(CUSTOM_MODEL);
292+
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME)))
293+
.thenReturn(null)
294+
.thenReturn(null)
295+
.thenReturn(CUSTOM_MODEL);
259296
when(mockModelDownloadService.getCustomModelDetails(
260297
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(null)))
261298
.thenReturn(Tasks.forResult(CUSTOM_MODEL));
@@ -360,7 +397,10 @@ public void getModel_updateBackground_noLocalModel() throws Exception {
360397

361398
@Test
362399
public void getModel_updateBackground_noLocalModel_error() throws Exception {
363-
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME))).thenReturn(null).thenReturn(CUSTOM_MODEL);
400+
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME)))
401+
.thenReturn(null)
402+
.thenReturn(null)
403+
.thenReturn(CUSTOM_MODEL);
364404
when(mockModelDownloadService.getCustomModelDetails(
365405
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(null)))
366406
.thenReturn(Tasks.forResult(CUSTOM_MODEL));
@@ -421,7 +461,10 @@ public void getModel_local_noLocalModel() throws Exception {
421461

422462
@Test
423463
public void getModel_local_noLocalModel_error() throws Exception {
424-
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME))).thenReturn(null).thenReturn(CUSTOM_MODEL);
464+
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME)))
465+
.thenReturn(null)
466+
.thenReturn(null)
467+
.thenReturn(CUSTOM_MODEL);
425468
when(mockModelDownloadService.getCustomModelDetails(
426469
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(null)))
427470
.thenReturn(Tasks.forResult(CUSTOM_MODEL));

0 commit comments

Comments
 (0)