Fix set user id for on-demand fatal

Author: themiswang

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java

@@ -38,6 +38,7 @@
 import com.google.firebase.crashlytics.internal.NativeSessionFileProvider;
 import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventLogger;
 import com.google.firebase.crashlytics.internal.metadata.LogFileManager;
+import com.google.firebase.crashlytics.internal.metadata.UserMetadata;
 import com.google.firebase.crashlytics.internal.model.CrashlyticsReport;
 import com.google.firebase.crashlytics.internal.persistence.FileStore;
 import com.google.firebase.crashlytics.internal.settings.Settings;
@@ -52,6 +53,7 @@
 import java.util.TreeSet;
 import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
+import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 
 public class CrashlyticsControllerTest extends CrashlyticsTestCase {
@@ -101,22 +103,33 @@ private class ControllerBuilder {
     private CrashlyticsNativeComponent nativeComponent = null;
     private AnalyticsEventLogger analyticsEventLogger;
     private SessionReportingCoordinator sessionReportingCoordinator;
+
+    private CrashlyticsBackgroundWorker backgroundWorker;
     private LogFileManager logFileManager = null;
 
+    private UserMetadata userMetadata = null;
+
     ControllerBuilder() {
       dataCollectionArbiter = mockDataCollectionArbiter;
       nativeComponent = mockNativeComponent;
 
       analyticsEventLogger = mock(AnalyticsEventLogger.class);
 
       sessionReportingCoordinator = mockSessionReportingCoordinator;
+
+      backgroundWorker = new CrashlyticsBackgroundWorker(new SameThreadExecutorService());
     }
 
     ControllerBuilder setDataCollectionArbiter(DataCollectionArbiter arbiter) {
       dataCollectionArbiter = arbiter;
       return this;
     }
 
+    ControllerBuilder setUserMetadata(UserMetadata userMetadata) {
+      this.userMetadata = userMetadata;
+      return this;
+    }
+
     public ControllerBuilder setNativeComponent(CrashlyticsNativeComponent nativeComponent) {
       this.nativeComponent = nativeComponent;
       return this;
@@ -153,13 +166,13 @@ public CrashlyticsController build() {
       final CrashlyticsController controller =
           new CrashlyticsController(
               testContext.getApplicationContext(),
-              new CrashlyticsBackgroundWorker(new SameThreadExecutorService()),
+              backgroundWorker,
               idManager,
               dataCollectionArbiter,
               testFileStore,
               crashMarker,
               appData,
-              null,
+              userMetadata,
               logFileManager,
               sessionReportingCoordinator,
               nativeComponent,
@@ -210,6 +223,26 @@ public void testFatalException_callsSessionReportingCoordinatorPersistFatal() th
         .persistFatalEvent(eq(fatal), eq(thread), eq(sessionId), anyLong());
   }
 
+  @Test
+  public void testOnDemandFatal_call() {
+    Thread thread = Thread.currentThread();
+    Exception fatal = new RuntimeException("Fatal");
+    Thread.UncaughtExceptionHandler exceptionHandler = mock(Thread.UncaughtExceptionHandler.class);
+    UserMetadata mockUserMetadata = mock(UserMetadata.class);
+    when(mockSessionReportingCoordinator.listSortedOpenSessionIds())
+        .thenReturn(new TreeSet<>(Collections.singleton(SESSION_ID)).descendingSet());
+
+    final CrashlyticsController controller =
+        builder()
+            .setLogFileManager(new LogFileManager(testFileStore))
+            .setUserMetadata(mockUserMetadata)
+            .build();
+    controller.enableExceptionHandling(SESSION_ID, exceptionHandler, testSettingsProvider);
+    controller.logFatalException(thread, fatal);
+
+    verify(mockUserMetadata).setNewSession(any(String.class));
+  }
+
   public void testNativeCrashDataCausesNativeReport() throws Exception {
     final String sessionId = "sessionId_1_new";
     final String previousSessionId = "sessionId_0_previous";

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java

@@ -214,7 +214,7 @@ public Task<Void> call() throws Exception {
 
                 doWriteAppExceptionMarker(timestampMillis);
                 doCloseSessions(settingsProvider);
-                doOpenSession(new CLSUUID(idManager).toString());
+                doOpenSession(new CLSUUID(idManager).toString(), isOnDemand);
 
                 // If automatic data collection is disabled, we'll need to wait until the next run
                 // of the app.
@@ -496,7 +496,7 @@ void openSession(String sessionIdentifier) {
         new Callable<Void>() {
           @Override
           public Void call() throws Exception {
-            doOpenSession(sessionIdentifier);
+            doOpenSession(sessionIdentifier, false);
             return null;
           }
         });
@@ -548,7 +548,7 @@ boolean finalizeSessions(SettingsProvider settingsProvider) {
    * Not synchronized/locked. Must be executed from the single thread executor service used by this
    * class.
    */
-  private void doOpenSession(String sessionIdentifier) {
+  private void doOpenSession(String sessionIdentifier, Boolean isOnDemand) {
     final long startedAtSeconds = getCurrentTimestampSeconds();
 
     Logger.getLogger().d("Opening a new session with ID " + sessionIdentifier);
@@ -566,6 +566,17 @@ private void doOpenSession(String sessionIdentifier) {
         startedAtSeconds,
         StaticSessionData.create(appData, osData, deviceData));
 
+    // If is on-demand fatal, we need to update the session id for userMetadata
+    // as well(since we don't really change the object to a new one for a new session).
+    // all the information in the previous session is still in memory, but we do need to
+    // manually writing them into persistence for the new session.
+    if (isOnDemand) {
+      final String currentSessionIdOnDemand = getCurrentSessionId();
+      if (currentSessionIdOnDemand != null) {
+        userMetadata.setNewSession(currentSessionIdOnDemand);
+      }
+    }
+
     logFileManager.setCurrentSession(sessionIdentifier);
     reportingCoordinator.onBeginSession(sessionIdentifier, startedAtSeconds);
   }

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java

@@ -41,7 +41,7 @@ public class UserMetadata {
 
   private final MetaDataStore metaDataStore;
   private final CrashlyticsBackgroundWorker backgroundWorker;
-  private final String sessionIdentifier;
+  private String sessionIdentifier;
 
   // The following references contain a marker bit, which is true if the data maintained in the
   // associated reference has been serialized since the last time it was updated.
@@ -77,6 +77,26 @@ public UserMetadata(
     this.backgroundWorker = backgroundWorker;
   }
 
+  /**
+   * Refresh the userMetadata to reflect the status of the new session. This API is mainly for
+   * on-demand fatal feature since we need to close and update to a new session. UserMetadata also
+   * need to make this update instead of updating session id, we also need to manually writing the
+   * into persistence for the new session.
+   */
+  public void setNewSession(String sessionId) {
+    synchronized (sessionIdentifier) {
+      sessionIdentifier = sessionId;
+      backgroundWorker.submit(
+          () -> {
+            if (getUserId() != null) {
+              metaDataStore.writeUserData(sessionIdentifier, getUserId());
+            }
+            // TODO(themis): adding feature rollouts later
+            return null;
+          });
+    }
+  }
+
   @Nullable
   public String getUserId() {
     return userId.getReference();