Merge pull request #2018 from bugsnag/PLAT-11930/optimize-ndk-breadcr…

…umb-type

Don't use strings for `BreadcrumbType` in NDK

bugsnag-plugin-android-ndk/src/main/java/com/bugsnag/android/ndk/NativeBridge.kt

@@ -1,6 +1,7 @@
 package com.bugsnag.android.ndk
 
 import android.os.Build
+import com.bugsnag.android.BreadcrumbType
 import com.bugsnag.android.NativeInterface
 import com.bugsnag.android.StateEvent
 import com.bugsnag.android.StateEvent.AddBreadcrumb
@@ -62,7 +63,15 @@ class NativeBridge(private val bgTaskService: BackgroundTaskService) : StateObse
     )
 
     external fun deliverReportAtPath(filePath: String)
-    external fun addBreadcrumb(name: String, type: String, timestamp: String, metadata: Any)
+    fun addBreadcrumb(name: String, type: String, timestamp: String, metadata: Any) {
+        val breadcrumbType = BreadcrumbType.values()
+            .find { it.toString() == type }
+            ?: BreadcrumbType.MANUAL
+
+        addBreadcrumb(name, breadcrumbType.toNativeValue(), timestamp, metadata)
+    }
+
+    private external fun addBreadcrumb(name: String, type: Int, timestamp: String, metadata: Any)
     external fun addMetadataString(tab: String, key: String, value: String)
     external fun addMetadataDouble(tab: String, key: String, value: Double)
     external fun addMetadataBoolean(tab: String, key: String, value: Boolean)
@@ -106,12 +115,14 @@ class NativeBridge(private val bgTaskService: BackgroundTaskService) : StateObse
                 event.section,
                 event.key ?: ""
             )
+
             is AddBreadcrumb -> addBreadcrumb(
                 event.message,
-                event.type.toString(),
+                event.type.toNativeValue(),
                 event.timestamp,
                 makeSafeMetadata(event.metadata)
             )
+
             NotifyHandled -> addHandledEvent()
             NotifyUnhandled -> addUnhandledEvent()
             PauseSession -> pausedSession()
@@ -121,11 +132,13 @@ class NativeBridge(private val bgTaskService: BackgroundTaskService) : StateObse
                 event.handledCount,
                 event.unhandledCount
             )
+
             is UpdateContext -> updateContext(event.context ?: "")
             is UpdateInForeground -> updateInForeground(
                 event.inForeground,
                 event.contextActivity ?: ""
             )
+
             is StateEvent.UpdateLastRunInfo -> updateLastRunInfo(event.consecutiveLaunchCrashes)
             is StateEvent.UpdateIsLaunching -> {
                 updateIsLaunching(event.isLaunching)
@@ -135,20 +148,24 @@ class NativeBridge(private val bgTaskService: BackgroundTaskService) : StateObse
                     bgTaskService.submitTask(TaskType.DEFAULT, this::refreshSymbolTable)
                 }
             }
+
             is UpdateOrientation -> updateOrientation(event.orientation ?: "")
             is UpdateUser -> {
                 updateUserId(event.user.id ?: "")
                 updateUserName(event.user.name ?: "")
                 updateUserEmail(event.user.email ?: "")
             }
+
             is StateEvent.UpdateMemoryTrimEvent -> updateLowMemory(
                 event.isLowMemory,
                 event.memoryTrimLevelDescription
             )
+
             is StateEvent.AddFeatureFlag -> addFeatureFlag(
                 event.name,
                 event.variant
             )
+
             is StateEvent.ClearFeatureFlag -> clearFeatureFlag(event.name)
             is StateEvent.ClearFeatureFlags -> clearFeatureFlags()
         }
@@ -227,4 +244,21 @@ class NativeBridge(private val bgTaskService: BackgroundTaskService) : StateObse
             }
         }
     }
+
+    /**
+     * Convert a [BreadcrumbType] to the value expected by the [addBreadcrumb] implementation. This
+     * is implemented as an exhaustive when so that any changes to the `enum` are picked up and/or
+     * don't directly impact the implementation.
+     */
+    @Suppress("MagicNumber") // introducing consts would reduce readability
+    private fun BreadcrumbType.toNativeValue(): Int = when (this) {
+        BreadcrumbType.ERROR -> 0
+        BreadcrumbType.LOG -> 1
+        BreadcrumbType.MANUAL -> 2
+        BreadcrumbType.NAVIGATION -> 3
+        BreadcrumbType.PROCESS -> 4
+        BreadcrumbType.REQUEST -> 5
+        BreadcrumbType.STATE -> 6
+        BreadcrumbType.USER -> 7
+    }
 }

bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c

@@ -387,36 +387,49 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_pausedSession(
 }
 
 JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addBreadcrumb(
-    JNIEnv *env, jobject _this, jstring name_, jstring crumb_type,
+    JNIEnv *env, jobject _this, jstring name_, jint crumb_type,
     jstring timestamp_, jobject metadata) {
 
   if (!bsg_jni_cache->initialized) {
     BUGSNAG_LOG("addBreadcrumb failed: JNI cache not initialized.");
     return;
   }
   const char *name = bsg_safe_get_string_utf_chars(env, name_);
-  const char *type = bsg_safe_get_string_utf_chars(env, crumb_type);
   const char *timestamp = bsg_safe_get_string_utf_chars(env, timestamp_);
 
-  if (name != NULL && type != NULL && timestamp != NULL) {
+  if (name != NULL && timestamp != NULL) {
     bugsnag_breadcrumb *crumb = calloc(1, sizeof(bugsnag_breadcrumb));
     bsg_strncpy(crumb->name, name, sizeof(crumb->name));
     bsg_strncpy(crumb->timestamp, timestamp, sizeof(crumb->timestamp));
-    if (strcmp(type, "user") == 0) {
-      crumb->type = BSG_CRUMB_USER;
-    } else if (strcmp(type, "error") == 0) {
+
+    // the values of crumb_type are defined in
+    // NativeBridge.BreadcrumbType.toNativeValue()
+    switch (crumb_type) {
+    case 0:
       crumb->type = BSG_CRUMB_ERROR;
-    } else if (strcmp(type, "log") == 0) {
+      break;
+    case 1:
       crumb->type = BSG_CRUMB_LOG;
-    } else if (strcmp(type, "navigation") == 0) {
+      break;
+    case 2:
+      crumb->type = BSG_CRUMB_MANUAL;
+      break;
+    case 3:
       crumb->type = BSG_CRUMB_NAVIGATION;
-    } else if (strcmp(type, "request") == 0) {
+      break;
+    case 4:
+      crumb->type = BSG_CRUMB_PROCESS;
+      break;
+    case 5:
       crumb->type = BSG_CRUMB_REQUEST;
-    } else if (strcmp(type, "state") == 0) {
+      break;
+    case 6:
       crumb->type = BSG_CRUMB_STATE;
-    } else if (strcmp(type, "process") == 0) {
-      crumb->type = BSG_CRUMB_PROCESS;
-    } else {
+      break;
+    case 7:
+      crumb->type = BSG_CRUMB_USER;
+      break;
+    default:
       crumb->type = BSG_CRUMB_MANUAL;
     }
 
@@ -428,7 +441,6 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addBreadcrumb(
     free(crumb);
   }
   bsg_safe_release_string_utf_chars(env, name_, name);
-  bsg_safe_release_string_utf_chars(env, crumb_type, type);
   bsg_safe_release_string_utf_chars(env, timestamp_, timestamp);
 }