Skip to content

Commit 4a4fbd2

Browse files
annzimmerannzimmer
authored
Adding deleteModel logging and retrieve download success events. (#2409)
* Adding deleteModel logging and retrieve download success events. * mocking issue * exception for permission errors * fix value number
1 parent 99c41d7 commit 4a4fbd2

File tree

10 files changed

+308
-16
lines changed

10 files changed

+308
-16
lines changed

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -460,15 +460,17 @@ public Task<Void> deleteDownloadedModel(@NonNull String modelName) {
460460
executor.execute(
461461
() -> {
462462
// remove all files associated with this model and then clean up model references.
463-
deleteModelDetails(modelName);
463+
boolean isSuccessful = deleteModelDetails(modelName);
464464
taskCompletionSource.setResult(null);
465+
eventLogger.logDeleteModel(isSuccessful);
465466
});
466467
return taskCompletionSource.getTask();
467468
}
468469

469-
private void deleteModelDetails(@NonNull String modelName) {
470-
fileManager.deleteAllModels(modelName);
470+
private boolean deleteModelDetails(@NonNull String modelName) {
471+
boolean isSuccessful = fileManager.deleteAllModels(modelName);
471472
sharedPreferencesUtil.clearModelDetails(modelName);
473+
return isSuccessful;
472474
}
473475

474476
/**

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

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,8 +229,8 @@ private Task<CustomModel> fetchDownloadDetails(String modelName, HttpURLConnecti
229229
modelName,
230230
httpResponseCode,
231231
"Too many requests to server please wait before trying again.",
232-
FirebaseMlException.INVALID_ARGUMENT);
233-
case HttpURLConnection.HTTP_SERVER_ERROR:
232+
FirebaseMlException.RESOURCE_EXHAUSTED);
233+
case HttpURLConnection.HTTP_INTERNAL_ERROR:
234234
return setAndLogException(
235235
modelName,
236236
httpResponseCode,
@@ -240,6 +240,17 @@ private Task<CustomModel> fetchDownloadDetails(String modelName, HttpURLConnecti
240240
modelName,
241241
errorMessage),
242242
FirebaseMlException.INTERNAL);
243+
case HttpURLConnection.HTTP_UNAUTHORIZED:
244+
case HttpURLConnection.HTTP_FORBIDDEN:
245+
return setAndLogException(
246+
modelName,
247+
httpResponseCode,
248+
String.format(
249+
Locale.getDefault(),
250+
"Issue while fetching model (%s); error message: %s",
251+
modelName,
252+
errorMessage),
253+
FirebaseMlException.PERMISSION_DENIED);
243254
default:
244255
return setAndLogException(
245256
modelName,
@@ -322,10 +333,18 @@ private Task<CustomModel> readCustomModelResponse(
322333
inputStream.close();
323334

324335
if (!downloadUrl.isEmpty() && expireTime > 0L) {
325-
return Tasks.forResult(
326-
new CustomModel(modelName, modelHash, fileSize, downloadUrl, expireTime));
336+
CustomModel model = new CustomModel(modelName, modelHash, fileSize, downloadUrl, expireTime);
337+
eventLogger.logModelInfoRetrieverSuccess(model);
338+
return Tasks.forResult(model);
327339
}
328-
return Tasks.forResult(null);
340+
eventLogger.logDownloadFailureWithReason(
341+
new CustomModel(modelName, modelHash, 0, 0L),
342+
false,
343+
ErrorCode.MODEL_INFO_DOWNLOAD_CONNECTION_FAILED.getValue());
344+
return Tasks.forException(
345+
new FirebaseMlException(
346+
"Model info could not be extracted from download response.",
347+
FirebaseMlException.INTERNAL));
329348
}
330349

331350
private static InputStream maybeUnGzip(InputStream input, String contentEncoding)

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

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.google.firebase.encoders.annotations.Encodable;
2525
import com.google.firebase.encoders.json.JsonDataEncoderBuilder;
2626
import com.google.firebase.ml.modeldownloader.internal.FirebaseMlLogEvent.EventName;
27+
import com.google.firebase.ml.modeldownloader.internal.FirebaseMlLogEvent.ModelDownloadLogEvent.ModelOptions.ModelInfo.ModelType;
2728
import java.lang.annotation.Retention;
2829
import java.lang.annotation.RetentionPolicy;
2930
import java.nio.charset.Charset;
@@ -59,7 +60,8 @@ public static Builder builder() {
5960
public enum EventName {
6061
UNKNOWN_EVENT(0),
6162
MODEL_DOWNLOAD(100),
62-
MODEL_UPDATE(101);
63+
MODEL_UPDATE(101),
64+
REMOTE_MODEL_DELETE_ON_DEVICE(252);
6365
private static final SparseArray<EventName> valueMap = new SparseArray<>();
6466

6567
private final int value;
@@ -68,6 +70,7 @@ public enum EventName {
6870
valueMap.put(0, UNKNOWN_EVENT);
6971
valueMap.put(100, MODEL_DOWNLOAD);
7072
valueMap.put(101, MODEL_UPDATE);
73+
valueMap.put(252, REMOTE_MODEL_DELETE_ON_DEVICE);
7174
}
7275

7376
EventName(int value) {
@@ -128,6 +131,9 @@ public abstract static class Builder {
128131
@Nullable
129132
public abstract ModelDownloadLogEvent getModelDownloadLogEvent();
130133

134+
@Nullable
135+
public abstract DeleteModelLogEvent getDeleteModelLogEvent();
136+
131137
@NonNull
132138
protected abstract Builder toBuilder();
133139

@@ -192,6 +198,7 @@ public int getValue() {
192198
public enum DownloadStatus {
193199
UNKNOWN_STATUS(0),
194200
EXPLICITLY_REQUESTED(1),
201+
MODEL_INFO_RETRIEVAL_SUCCEEDED(3),
195202
MODEL_INFO_RETRIEVAL_FAILED(4),
196203
SCHEDULED(5),
197204
DOWNLOADING(6),
@@ -205,6 +212,7 @@ public enum DownloadStatus {
205212
static {
206213
valueMap.put(0, UNKNOWN_STATUS);
207214
valueMap.put(1, EXPLICITLY_REQUESTED);
215+
valueMap.put(3, MODEL_INFO_RETRIEVAL_SUCCEEDED);
208216
valueMap.put(4, MODEL_INFO_RETRIEVAL_FAILED);
209217
valueMap.put(5, SCHEDULED);
210218
valueMap.put(6, DOWNLOADING);
@@ -327,6 +335,34 @@ public abstract static class Builder {
327335
}
328336
}
329337

338+
@AutoValue
339+
public abstract static class DeleteModelLogEvent {
340+
@NonNull
341+
public static Builder builder() {
342+
return new AutoValue_FirebaseMlLogEvent_DeleteModelLogEvent.Builder()
343+
.setModelType(ModelType.CUSTOM)
344+
.setIsSuccessful(true);
345+
}
346+
347+
@ModelType
348+
public abstract int getModelType();
349+
350+
public abstract boolean getIsSuccessful();
351+
352+
/** Builder for {@link DeleteModelLogEvent}. */
353+
@AutoValue.Builder
354+
public abstract static class Builder {
355+
@NonNull
356+
public abstract Builder setModelType(@ModelType int value);
357+
358+
@NonNull
359+
public abstract Builder setIsSuccessful(boolean value);
360+
361+
@NonNull
362+
public abstract DeleteModelLogEvent build();
363+
}
364+
}
365+
330366
@AutoValue.Builder
331367
public abstract static class Builder {
332368

@@ -339,6 +375,9 @@ public abstract static class Builder {
339375
@Nullable
340376
public abstract Builder setModelDownloadLogEvent(@Nullable ModelDownloadLogEvent value);
341377

378+
@Nullable
379+
public abstract Builder setDeleteModelLogEvent(@Nullable DeleteModelLogEvent value);
380+
342381
@NonNull
343382
public abstract FirebaseMlLogEvent build();
344383
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.firebase.FirebaseApp;
2626
import com.google.firebase.ml.modeldownloader.BuildConfig;
2727
import com.google.firebase.ml.modeldownloader.CustomModel;
28+
import com.google.firebase.ml.modeldownloader.internal.FirebaseMlLogEvent.DeleteModelLogEvent;
2829
import com.google.firebase.ml.modeldownloader.internal.FirebaseMlLogEvent.EventName;
2930
import com.google.firebase.ml.modeldownloader.internal.FirebaseMlLogEvent.ModelDownloadLogEvent;
3031
import com.google.firebase.ml.modeldownloader.internal.FirebaseMlLogEvent.ModelDownloadLogEvent.DownloadStatus;
@@ -96,6 +97,16 @@ void logModelInfoRetrieverFailure(CustomModel model, ErrorCode errorCode) {
9697
logModelInfoRetrieverFailure(model, errorCode, NO_FAILURE_VALUE);
9798
}
9899

100+
void logModelInfoRetrieverSuccess(CustomModel model) {
101+
logDownloadEvent(
102+
model,
103+
ErrorCode.NO_ERROR,
104+
false,
105+
/* shouldLogExactDownloadTime= */ false,
106+
DownloadStatus.MODEL_INFO_RETRIEVAL_SUCCEEDED,
107+
FirebaseMlLogEvent.NO_INT_VALUE);
108+
}
109+
99110
void logModelInfoRetrieverFailure(CustomModel model, ErrorCode errorCode, int httpResponseCode) {
100111
logDownloadEvent(
101112
model,
@@ -148,6 +159,25 @@ private boolean isStatsLoggingEnabled() {
148159
return sharedPreferencesUtil.getCustomModelStatsCollectionFlag();
149160
}
150161

162+
public void logDeleteModel(boolean success) {
163+
if (!isStatsLoggingEnabled()) {
164+
return;
165+
}
166+
167+
try {
168+
eventSender.sendEvent(
169+
FirebaseMlLogEvent.builder()
170+
.setDeleteModelLogEvent(
171+
DeleteModelLogEvent.builder().setIsSuccessful(success).build())
172+
.setEventName(EventName.REMOTE_MODEL_DELETE_ON_DEVICE)
173+
.setSystemInfo(getSystemInfo())
174+
.build());
175+
} catch (RuntimeException e) {
176+
// Swallow the exception since logging should not break the SDK usage
177+
Log.e(TAG, "Exception thrown from the logging side", e);
178+
}
179+
}
180+
151181
private void logDownloadEvent(
152182
CustomModel customModel,
153183
ErrorCode errorCode,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,9 @@ public synchronized void deleteOldModels(
238238
* destination for a model in this method.
239239
*/
240240
@WorkerThread
241-
public synchronized void deleteAllModels(@NonNull String modelName) {
241+
public synchronized boolean deleteAllModels(@NonNull String modelName) {
242242
File modelFolder = getModelDirUnsafe(modelName);
243-
deleteRecursively(modelFolder);
243+
return deleteRecursively(modelFolder);
244244
}
245245

246246
/**

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,7 @@ public void listDownloadedModels_returnsModelList() throws Exception {
739739
@Test
740740
public void deleteDownloadedModel() throws Exception {
741741
doNothing().when(mockPrefs).clearModelDetails(eq(MODEL_NAME));
742-
doNothing().when(mockFileManager).deleteAllModels(eq(MODEL_NAME));
742+
when(mockFileManager.deleteAllModels(eq(MODEL_NAME))).thenReturn(true);
743743

744744
TestOnCompleteListener<Void> onCompleteListener = new TestOnCompleteListener<>();
745745
Task<Void> task = firebaseModelDownloader.deleteDownloadedModel(MODEL_NAME);
@@ -749,6 +749,7 @@ public void deleteDownloadedModel() throws Exception {
749749
assertThat(task.isComplete()).isTrue();
750750
verify(mockPrefs, times(1)).clearModelDetails(eq(MODEL_NAME));
751751
verify(mockFileManager, times(1)).deleteAllModels(eq(MODEL_NAME));
752+
verify(mockEventLogger, times(1)).logDeleteModel(eq(true));
752753
}
753754

754755
@Test

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

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,89 @@ public void downloadService_badRequest() {
329329
eq(HttpURLConnection.HTTP_BAD_REQUEST));
330330
}
331331

332+
@Test
333+
public void downloadService_forbidden() {
334+
String downloadPath =
335+
String.format(CustomModelDownloadService.DOWNLOAD_MODEL_REGEX, "", PROJECT_ID, MODEL_NAME);
336+
stubFor(
337+
get(urlEqualTo(downloadPath))
338+
.withHeader(
339+
CustomModelDownloadService.INSTALLATIONS_AUTH_TOKEN_HEADER,
340+
equalTo(INSTALLATION_TOKEN))
341+
.withHeader(
342+
CustomModelDownloadService.CONTENT_TYPE,
343+
equalTo(CustomModelDownloadService.APPLICATION_JSON))
344+
.withHeader(CustomModelDownloadService.IF_NONE_MATCH_HEADER_KEY, equalTo(MODEL_HASH))
345+
.willReturn(
346+
aResponse()
347+
.withStatus(HttpURLConnection.HTTP_FORBIDDEN)
348+
.withBody(
349+
"{\"status\":\"PERMISSION_DENIED\",\"message\":\"Request not valid\"}")));
350+
351+
CustomModelDownloadService service =
352+
new CustomModelDownloadService(
353+
installationsApiMock, directExecutor, API_KEY, TEST_ENDPOINT, mockEventLogger);
354+
355+
Task<CustomModel> modelTask = service.getCustomModelDetails(PROJECT_ID, MODEL_NAME, MODEL_HASH);
356+
357+
Assert.assertTrue(modelTask.getException() instanceof FirebaseMlException);
358+
Assert.assertEquals(
359+
((FirebaseMlException) modelTask.getException()).getCode(),
360+
FirebaseMlException.PERMISSION_DENIED);
361+
362+
WireMock.verify(
363+
getRequestedFor(urlEqualTo(downloadPath))
364+
.withHeader(
365+
CustomModelDownloadService.INSTALLATIONS_AUTH_TOKEN_HEADER,
366+
equalTo(INSTALLATION_TOKEN)));
367+
verify(mockEventLogger, times(1))
368+
.logModelInfoRetrieverFailure(
369+
any(),
370+
eq(ErrorCode.MODEL_INFO_DOWNLOAD_UNSUCCESSFUL_HTTP_STATUS),
371+
eq(HttpURLConnection.HTTP_FORBIDDEN));
372+
}
373+
374+
@Test
375+
public void downloadService_internalError() {
376+
String downloadPath =
377+
String.format(CustomModelDownloadService.DOWNLOAD_MODEL_REGEX, "", PROJECT_ID, MODEL_NAME);
378+
stubFor(
379+
get(urlEqualTo(downloadPath))
380+
.withHeader(
381+
CustomModelDownloadService.INSTALLATIONS_AUTH_TOKEN_HEADER,
382+
equalTo(INSTALLATION_TOKEN))
383+
.withHeader(
384+
CustomModelDownloadService.CONTENT_TYPE,
385+
equalTo(CustomModelDownloadService.APPLICATION_JSON))
386+
.withHeader(CustomModelDownloadService.IF_NONE_MATCH_HEADER_KEY, equalTo(MODEL_HASH))
387+
.willReturn(
388+
aResponse()
389+
.withStatus(HttpURLConnection.HTTP_INTERNAL_ERROR)
390+
.withBody(
391+
"{\"status\":\"INTERNAL\",\"message\":\"Request cannot reach server\"}")));
392+
393+
CustomModelDownloadService service =
394+
new CustomModelDownloadService(
395+
installationsApiMock, directExecutor, API_KEY, TEST_ENDPOINT, mockEventLogger);
396+
397+
Task<CustomModel> modelTask = service.getCustomModelDetails(PROJECT_ID, MODEL_NAME, MODEL_HASH);
398+
399+
Assert.assertTrue(modelTask.getException() instanceof FirebaseMlException);
400+
Assert.assertEquals(
401+
((FirebaseMlException) modelTask.getException()).getCode(), FirebaseMlException.INTERNAL);
402+
403+
WireMock.verify(
404+
getRequestedFor(urlEqualTo(downloadPath))
405+
.withHeader(
406+
CustomModelDownloadService.INSTALLATIONS_AUTH_TOKEN_HEADER,
407+
equalTo(INSTALLATION_TOKEN)));
408+
verify(mockEventLogger, times(1))
409+
.logModelInfoRetrieverFailure(
410+
any(),
411+
eq(ErrorCode.MODEL_INFO_DOWNLOAD_UNSUCCESSFUL_HTTP_STATUS),
412+
eq(HttpURLConnection.HTTP_INTERNAL_ERROR));
413+
}
414+
332415
@Test
333416
public void downloadService_tooManyRequest() {
334417
String downloadPath =
@@ -357,7 +440,7 @@ public void downloadService_tooManyRequest() {
357440
Assert.assertTrue(modelTask.getException() instanceof FirebaseMlException);
358441
Assert.assertEquals(
359442
((FirebaseMlException) modelTask.getException()).getCode(),
360-
FirebaseMlException.INVALID_ARGUMENT);
443+
FirebaseMlException.RESOURCE_EXHAUSTED);
361444

362445
WireMock.verify(
363446
getRequestedFor(urlEqualTo(downloadPath))
@@ -396,7 +479,8 @@ public void downloadService_authenticationIssue() {
396479

397480
Assert.assertTrue(modelTask.getException() instanceof FirebaseMlException);
398481
Assert.assertEquals(
399-
((FirebaseMlException) modelTask.getException()).getCode(), FirebaseMlException.INTERNAL);
482+
((FirebaseMlException) modelTask.getException()).getCode(),
483+
FirebaseMlException.PERMISSION_DENIED);
400484
Assert.assertTrue(
401485
modelTask
402486
.getException()

0 commit comments

Comments
 (0)